The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

Basic OOP question

The requirement in my project is creation of itinearay ..which will have a name,category(like adventure,classisc),noofdays(eg 6 day itinerary),and activities assigned to each day..so..to design this situation when user creates an itinerary,i need to create an itinerary object with name,category and noofdays(my understnding is that these are invariant of class)..my question is where to check if data supplied by user is incorrect(eg 0 nofodays is invalid)..in constructor..if so,if data is invalid..what needs to be done.. raise exception and destroy object? or use a factory class to create object..which will check data provided and if everything is ok,then create itinerary object..I am using C#..
thks
vishy Send private email
Tuesday, May 23, 2006
 
 
I like to reserve using exceptions only for events that the program thinks shouldn't happen and which the program doesn't want to handle.

I'd say that user's entering bad data is a normal situation which the program should handle.

I'd therefore prefer to validate the data in the factory method, and not create the object at all if the data is invalid.
Christopher Wells Send private email
Tuesday, May 23, 2006
 
 
so,what should i do in constructor..if a value passed is invalid..i do not want object should be created..
thks
vishy Send private email
Tuesday, May 23, 2006
 
 
By the time you're in the constructor, if you don't like the parameters then (all you can do is) throw an exception; but to avoid throwing an exception, I'd validate the parameters (in the factory method) before invoking the constructor.

I wouldn't go to all this bother (having a factory method to validate parameters) if the parameters were coming from other modules in my program (because I'm willing to assert that other modules in *my* program will be passing me sane paramaters); but when the parameters are coming from the *user*, then I wouldn't consider bad input parameters as "exceptional"
Christopher Wells Send private email
Tuesday, May 23, 2006
 
 
The class that you are trying to instantiate should be responsible for validating the data.

My solution would be to have a _simple_ Factory Method on the class in question which populates a unique id field in the object.  Then you've got an object all set to validate your data.  If the data is invalid, let your new object tell you.  Having data validation outside of a business object often leads to code duplication.
 
Example of simple Factory Method:

class BusinessClass
{
  private Guid id = Guid.Empty;
  // Private constuctor
  private BusinessClass(Guid id)
  {
    this.id = id;
  }
  // Factory method
  public static BusinessClass NewBusinessClass()
  {
    return new BusinessClass(Guid.NewGuid);
  }
}
sensible Send private email
Tuesday, May 23, 2006
 
 
so sensible,i should hav a factory method in a class..is this pattern used mostly for creation of objects..or how to create an object is determined by requirement..if so,how?
vishy Send private email
Tuesday, May 23, 2006
 
 
In the example I gave, the Factory Method is designed specifically to create new and empty instances of the class.  Once you have a empty instance, you can set the other properties defined byt the class thereby allowing the object's inherent validation logic to run its rule over the data.

Of course, you can have a multitude of Factory Methods that accept various arguments and instantiate the class in a variety of ways.  I was just illustrating my point with a basic and practical example.
sensible Send private email
Tuesday, May 23, 2006
 
 
You should validate the input at the input location.  So if the user enters the data on a form, that's where the validation should occur.  The user should get a nice error saying the number of days is invalid.

In the class constructor, or factor, or even the set accessor of the class you should raise an exception if the value passed is not valid.  This *should* never happen because you validate the input but then that's why it's an exception.
Almost H. Anonymous Send private email
Tuesday, May 23, 2006
 
 
I agree with Almost. The best way to solve this problem is to restrict your users power of input. This specific example should have a listbox or scroll control with only the valid inputs listed. If you cannot do this, DO NOT use your constructor for this, a property method with validation code is more maintainable. The factory idea is somewhat of an overkill and will drive Erick Gamma to tears since he intended factories to produce instances of different classes based on input.

If you are writing a three tiered application, you don’t want the input to go all the way to the middle tier just to fail, rather handle it on the front end.

You have stumbled on a common problem that has kept many a developer guessing and you are most probably going to end up writing a line of non-oo code to check this.
Marius Mans Send private email
Tuesday, May 23, 2006
 
 
Hmmmmm. I think I disagree with the basic modeling of your class. I don't think you should have a noofdays field at all.

Instead, I would think of an Itinerary as an ordered collection of Event objects. Each event would have a start time and an end time (or a duration). To calculate the duration of the entire Itinerary, I'd use the startTime of the first Event and the endTime of the last event. If fractional days caused problems, I'd just round up to the nearest whole day.

I'd offer two different constructors for the Itinerary class: a no-arg constructor for creating an empty Itinerary, and a constructor that accepts a List of Event objects.
BenjiSmith Send private email
Tuesday, May 23, 2006
 
 
Also...I don't see the point of sensible's Factory.

I generally only use a factory when I want to hide some detail about the construction of the object. For example, if I want to code against an interface, and the calling code shouldn't have to know anything about the implementation class.

But this example...I just don't get it. Why not just explicitly call the constructor?
BenjiSmith Send private email
Tuesday, May 23, 2006
 
 
So there's basically two schools of thought represented here.

One is AHA's point of view -- validate in the routine that WOULD call the 'new', and only create the object if you've got good data to create it with.

The other is 'sensible's approach -- each object should know how to validate its own data.  (I kind of agree with this approach, personally).  Thus you want a 'light' constructor, which creates the object with as few properties as necessary.  Then pass the suspect data to this new object, which is responsible for determining if it's 'good' or not.

Having said all this, I still prefer doing as little as possible in a constructor, but what do you do after an object's "Init" method (with bad data)?  If it was really bad data, now you have a 'bad' object, which must indicate this in some way to the 'creator', so the 'creator' can do whatever it wants to do with a 'bad' object.  And you probably still need to prompt the user for better data at some point.

Either approach can be used.  AHA's approach lets you do a LOT in the constructor, and means you don't have to deal with 'bad' instances of objects.
AllanL5
Tuesday, May 23, 2006
 
 
Anything the user can input will be wrong - that being said what follows is not a drop in replacement for commen sense validation.  Just like you would want to validate a date before submission on a form you want some things to fail long before you're making a call to some sub-system deep in the system.

A constructors purpose is to construct an object in memory.  If for some reason you are unable to construct the object then you should throw an expection; in much the same way you will get an exception for allocating too much memory without it being available.  A constructors purpose is to provide custom, detailed support on the "how" to build the object at hand alone with its own special invariants if they exist.  If you find yourself doing more than that then it is *my* humble opinion that you need to rethink what it is your doing because you're doing it wrong.
Nobody special
Tuesday, May 23, 2006
 
 
I am doing test-first type of development..and i feel constructor should raise exception if data provided is not valid..its class responibility to make sure it is in valid state..also,when user inputs data..the validation will be done using class only(i am developing windows forms app)..yes i will do some validation on controls too..but its secondary..
vishy Send private email
Wednesday, May 24, 2006
 
 
"The factory idea is somewhat of an overkill and will drive Erick Gamma to tears since he intended factories to produce instances of different classes based on input".

I think you'll find that a Factory Method is simply a method responsible for manufactoring an object.  The design patterns book goes into one example of how this may be done.  There are many varieties of the pattern.  My example Factory Method has the sole responsibility of manufactoring a brand new business object.  If you look closer you'll see the working code is the same as the factory methods from the book, only I've removed the need for an independent Factory class which would be overkill for my example.

The idea behind the Factory Method is decouple the client code from a particular concrete Type.  Once a class can only be instantiated from static factory methods (whether in-class or through a factory class) then you have achieved this decoupling.  If the object creation process needs to change simply alter the factory method, thereby avoiding any changes to clients that would otherwise have had to call "new BusinessClass(id)".
sensible Send private email
Wednesday, May 24, 2006
 
 
> The class that you are trying to instantiate should be responsible for validating the data.

Yes, why not.

> My solution would be to have a _simple_ Factory Method on the class in question which populates a unique id field in the object.  Then you've got an object all set to validate your data.

No, because now you have an instantiated object which contains no useful data, which violates one of the invariants (e.g "all instances of this class must have a non-empty itineray"). So instead I'd put the validation in a *static* method of the class (called a "factory method") which constructs the object.

class Class
{
  public static Class Create(Data data)
  {
    //validate the input data
    //compain and return null if data is unworthy
    //otherwise construct and return the perfect object
    return new Class(data);
  }
  public Class(Data data)
  : m_data(data)
  {
    //if public then validate again, and throw if bad
  }
  Data m_data;
};
Christopher Wells Send private email
Wednesday, May 24, 2006
 
 
So...sensible, do you use factory classes to produce all of your objects? In which cases do you think it's more appropriate to call a constructor directly than to use a factory class?
BenjiSmith Send private email
Wednesday, May 24, 2006
 
 
The architecture I use leads me towards using factories to create all of my business objects.  Sometimes my objects need to hop across a network to collect some initial data and I like to keep UI free from that sort of detail.  The factory methods I employ allow me to achieve all this in a consistent manner.  Calling constructors from UI code will force the UI to know far too much about my business objects.  The example I gave is a simple example that doesn't demonstrate the benefits of the factory method.
sensible Send private email
Thursday, May 25, 2006
 
 
Gotcha.

Though why are you creating business objects in the UI code? Usually, I create all of my business objects in the service layer and then--only when they're ready to be displayed--I pass the objects over to the UI code for rendering.

I agree with you that the UI shouldn't have to call constructors, but I'd argue that that UI shouldn't be creating objects at all.
BenjiSmith Send private email
Thursday, May 25, 2006
 
 
I agree with Christopher Wells that you should not construct an object that isn't valid, and a static factory method is one solution, but I don't agree with the comment in his example code "compain and return null if data is unworthy". I believe that one should generally avoid returning null when the creation of an object is requested - this leads to messy checks and handling for null among the process code. Better rather to throw an exception which can be handled cleanly away from the main action - and which can be done from the constructor, making a factory method redundant - OR use the Null Object pattern with a factory method. The Null Object pattern isn't always suitable (probably not in this particular instance), but can make an effective and elegant solution where appropriate.
Dave Lorde Send private email
Friday, May 26, 2006
 
 
> I believe that one should generally avoid returning null when the creation of an object is requested - this leads to messy checks and handling for null among the process code.

Generally, yes. In this specific case I felt that bad user input isn't exceptional, and I really do prefer to reserve exceptions for unexpected conditions (assertion failures). Here I'm assuming that Class::Create is well-documented as returning null if the input data is bad: so if the caller tries to dereference a returned null, *that* would be the exception.

> Better rather to throw an exception which can be handled cleanly away from the main action

I'm expecting this factory method to be called from UI: i.e. this method is already on the very boundary edge of the system, whose sole purpose is to interface the user to the rest of the process.
Christopher Wells Send private email
Friday, May 26, 2006
 
 
I have created a static factory..if unable to create object then this method returns null..But I also want the user to know why,object was not created so,I am thinking of raising an event(.net here,c#) which have information like what data was bad..which UI can handle..

It is amazing the kind of response I got in this question,it has helped me a lot..thks all..
vishy
Saturday, May 27, 2006
 
 
Defining an event in a class is especially useful when the class is in a lower-level project (module or layer), which wants to invoke a method in an unknown-to-it higher-level class.

An alternative might be is to pass-in or pass-out another object instance as a parameter, for example ...

class Class
{
  /// This method constructs and returns a Class instance if the data is good, otherwise it returns null and invokes the Tell method of the passed-in User instance
  public static Class Create(Data data,User user)
  {
    //validate the input data:
    //complain and return null if data is unworthy
    if (dataNumDays == 0)
    {
      user.Tell("You must specify the number of days.");
      return null;
    }
    //otherwise construct and return the perfect object
    return new Class(data);
  }
  prvate Class(Data data)
  : m_data(data)
  {
  }
  Data m_data;
};

... or perhaps ...

class Class
{
  /// This method constructs and returns a Class instance and a null userMessage if the data is good, otherwise it returns a null Class instance and a non-null userMessage
  public static Class Create(Data data,out string userMessage)
  {
    userMessage = null;
    //validate the input data:
    //complain and return null if data is unworthy
    if (dataNumDays == 0)
    {
      userMessage = "You must specify the number of days."
      return 0;
    }
    //otherwise construct and return the perfect object
    return new Class(data);
  }
  prvate Class(Data data)
  : m_data(data)
  {
  }
  Data m_data;
};
Christopher Wells Send private email
Saturday, May 27, 2006
 
 
hi christopher,thks for both the ideas...i think i will go with raising event..this event will also tell what is wrong..then the UI/application layer(presenter in my case,i am using MVP pattern)..will handle it..
cheers!!
vishy
Saturday, May 27, 2006
 
 
If a method is called ValidateDataAndCreateObjectIfValid() then I'm sure something is wrong.

If it is doing two things (validating data and creating an object) but the name only mentions one detail, then it has been misnamed.

On the other hand, if you have a class DataFromUser with a method called validate() then Create can take a UserData object, assume that it's valid, and even raise an exception if not because that would be violating the obvious precondition - that the data is valid. The DataFromUser class's method is useful in many situations, both in doing something weith the data, and for letting the user interface include more real-time indications of problems.

Personally, I prefer many small classes to a few big classes, unless there's a good reason why it's necessary.

Sunday, May 28, 2006
 
 
> class DataFromUser with a method called validate()

That's good. The one down-side is that you may have duplicated the validation rules: one set of validation rules defined in the DataFromUser class, and another very similar set of assertion/invariant rules defined in the BusinessObjectCreatedUsingDataFromUser class.
Christopher Wells Send private email
Monday, May 29, 2006
 
 
... so perhaps put the rule as a static method in the business class only:

class UserData
{
  public int NumDays { get { return m_numDays; } }
  public bool IsValid { return { return Class::IsValid(this); } }
  ... etc ...
}

class Class
{
  /// This method validates user data
  public static bool IsValid(UserData data)
  {
    //validate the input data and
    //return true iff the data is worthy
    return (data.NumDays != 0);
  }
  /// This constructor throws an exception if it's passed-in data which doesn't pass the tests defined by the IsValid method
  public Class(UserData data)
  : m_data(data)
  {
    //validate the input data and
    //throw exception if data is unworthy
    if (!IsValid(data))
      throw new UserDataIsUnworthyException(data);
  }
  UserData m_data;
};
Christopher Wells Send private email
Monday, May 29, 2006
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz