Skip to main content
Software Engineering

Return to Question

Tweeted twitter.com/#!/StackProgrammer/status/469467993299513344
added 1034 characters in body
Source Link
BVernon
BVernon
  • 503
  • 3
  • 12

Edit: More Info

I realize now I left out a an important detail. My team lead is very, very big on requiring all data needed to initialize a proper type to be passed into its constructor (and not allowing a default constructor), and putting validation logic in the properties' set accessors. He feels this is the best way to prevent bad data from getting into the database. I don't particularly like this approach, however he's worked at some places where lack of good validation caused serious, serious problems.

So that's why I use a factory method... but the drawback of that is that type casting is required when the extended properties have to be worked with. Another drawback is that if I make the properties virtual then now, because of our validation approach, I have to call virtual properties from the constructor. Actually I ended up doing just that of necessity and practicality, but I'm didn't mention it in the beginning because doing so was a 'work around' to the actual design that I wanted to ask about.

Edit: More Info

I realize now I left out a an important detail. My team lead is very, very big on requiring all data needed to initialize a proper type to be passed into its constructor (and not allowing a default constructor), and putting validation logic in the properties' set accessors. He feels this is the best way to prevent bad data from getting into the database. I don't particularly like this approach, however he's worked at some places where lack of good validation caused serious, serious problems.

So that's why I use a factory method... but the drawback of that is that type casting is required when the extended properties have to be worked with. Another drawback is that if I make the properties virtual then now, because of our validation approach, I have to call virtual properties from the constructor. Actually I ended up doing just that of necessity and practicality, but I'm didn't mention it in the beginning because doing so was a 'work around' to the actual design that I wanted to ask about.

Source Link
BVernon
BVernon
  • 503
  • 3
  • 12

Is this actually good OO design?

Context

I have Specification class with several properties including: StartDate, EndDate, DaysToComplete, DateToComplete.

A specification contains a list of required courses (i.e. a company requires employees to take training... perhaps a technical, diversity, or an ethics class). Based on a specification, courses will be mandated to employees.

Validation Concerns

StartDate and EndDate refer to the window of time in which a company can mandate a class to a user. DaysToComplete is how many days the employee has to compete training from the time it's mandated to them, whereas DateToComplete is the last day they have to complete the course.

  • StartDate must always be specified
  • DaysToComplete and DateToComplete are mutually exclusive... one and only one must be specified.
  • If EndDate is not specified, then DaysToComplete must be used
  • If StartDate = EndDate, then DateToComplete must be used

And there are other obvious validation rules such as the fact that you wouldn't have an EndDate greater than the DateToComplete since you wouldn't mandate a course after it already had to be completed.

The Problem

It seems as though there is a case here for having a base class and 4 derived classes with the following properties (i.e. this is how I was instructed to and did design the objects):

  • SpecificationBase: StartDate
  • PerpetualSpecification: DaysToComplete
  • OneTimeSpecification*: DateToComplete
  • FixedSpecification: EndDate, CompletionDate
  • SlidingSpecification: EndDate, DaysToComplete

*When a user enters an EndDate that equals the StartDate they must use DateToComplete. In this case the EndDate is not actually saved though.

So we're using inheritance to do two things here: encapsulate validation rules and not expose properties that don't exist for particular scenarios.

As far as validation goes, from my perspective, the rules are always the same for all scenarios. Yes, certain properties don't apply in all scenarios but for the ones that do the validation logic really doesn't change. The argument, however, is that it is easier to comprehend the validation logic this way because putting it in all in the base class would be too complicated (writing a factory method to choose the right type based on user input and create it with reflection was, by comparison, considered not a big deal at all).

And as for hiding unused properties (or as my team lead insists: not exposing properties that don't exist), the argument started out as being an OO design principle that data and behavior should be coupled... until I pointed out that it would still be coupled in the base class without the derived classes (not to mention the data isn't decoupled in the database either).

I highly respect my team lead and from a purely philosophical/idealistic point of view this design seems great, but in practice it seems to create more problems than it solves:

  • Had to create a factory method since we just get input from users without making them specify the type
  • 3rd party tools that rely on reflection can't work with a list of different types with different properties so I have to create a proxy (or cheat and just put the data into an anonymous type and hope I'm never asked to provide a way to update the data from a grid control)
  • Because consuming code only uses the factory method, it doesn't know what specific type it has or which properties are on it. This means if/then statements to test for what type I'm working with before trying to access a property that might not exist.

I'm sorry for the incredibly long essay, but I just wanted to provide ample context and background before asking my question: Is this 'really' good OO design? I feel like good OO design should make life easier... not harder. What am I missing here?