Business rules - where do they go in OOP?

Question

I have a class : Schedule.

public class Schedule {

private int locationNum;
private int cost;
private String costReason; 
private Date weekOfChange;
private Date dayOfChange;
private String changeReason; 

// and all those getters and setters

public Schedule(int locationNum, int cost, String costReason, Date weekOfChange, Date dayOfChange, String changeReason) throws ApplicationException {
//change is all or nothing - all attributes are present or none
if((weekOfChange!=null && dayOfChange!=null && changeReason!=null) || (weekOfChange==null  && dayOfChange == null && changeReason == null))  {
this.weekOfChange = weekOfChange;
this.dayOfChange = dayOfChange;
this.changeReason = changeReason;
}
else { throw new ApplicationException();}
//similary another if block to ensure that if cost is specified 
//then there exists the corresponding reason code for it.
}
}

So far, I am liking my Schedule class. However, I am not done with the checks, I would have to do some other checks:

• is the locationNum a valid store number in the database.
• is changeReason text one of those 6 different changeReason codes in the database.
• etc etc....

Typically, I would not write these in Schedule class, coz obviously, I cannot invoke DAOs from this class. So, I would have a business layer and some kind of validator class, accepting an object of type Schedule and performing a bunch of database validations in sequence and collecting errors for display / whatever.

Now, here are my questions:

1. If you were to treat Schedule as a POJO and argue that an object is not responsible for validating itself - I would have to move all of the code in the constructor to the business layer's validator class. But, if I were to do this, isn't Schedule anemic? Is this what they call violation of Single Responsibility principle?
2. Lets presume I moved the code I have in my constructor to a business layer class so that all kinds of validations are now in my busines layer. Lets assume that someone changes dayOfChange to NULL in my datastore and now I am loading objects from my database. Now, with this kind of an object, my application can break, wouldn't it? coz I would have written code assuming that validation rules are met. I guess my question is getting confusing, but the point I am trying to make is that in this context I would rather have these checks done in constructor to ensure integrity of the data hidden by Schedule class.
3. How is it done usually? What is the best practice?

Appreciate your involvement in this discussion.

Answer 1

Why is locationNum an int when in fact you refer to an instance of another class, say Location? If you'd use proper object references you could validate them in Schedule and configure the ORM (if any) to enforce referential integrity.

Answer 2

If you were to treat Schedule as a POJO and argue that an object is not responsible for validating itself - I would have to move all of the code in the constructor to the business layer's validator class. But, if I were to do this, isn't Schedule anemic? Is this what they call violation of Single Responsibility principle?

POJO only means that you don't have to derive from a particular class or use a byte code enhancer to get desired functionality like OR mapping. It does NOT mean that the objects should be anemic with no functionality.

Now, with this kind of an object, my application can break, wouldn't it? coz I would have written code assuming that validation rules are met. I guess my question is getting confusing, but the point I am trying to make is that in this context I would rather have these checks done in constructor to ensure integrity of the data hidden by Schedule class.

Who says you cannot call rules defined in a separate class from the constructor of your Schedule class? If it needs access to non-public fields, you could make them package-private and have the rules class in the same package - or pass them as parameters to the rule.

How is it done usually? What is the best practice?

Whether or not you move validation rules to separate classes should depend on your application's needs, not on misunderstood buzzwords.

Good reasons to have separate rule classes would be:

• The rules are so many and so complex that putting them all into the Schedule class would make it too big.
• You do not always want to apply all the rules.
• The rules can be reused to operate on different classes that are not necessarily in the same inheritance hierarchy (i.e. they work through an interface).

Answer 3

to answer your questions :

1) I don't think your class will be anemic, it will rather be a DTO for the moment, I see no problem with this.

2) If your validation is in the business layer, it doesn't mean you'll be getting an invalid object, because the validation will still be there, normally your validation process will somehow complain about an InvalidDataException or something and won't allow you to use the "corrupted" object. If the null date is not an acceptable value ( null could not normally get into the database ), then the business/dao layer should catch this error and prevent the object to get used by your app.

Answer 4

It looks like there is two distinct behaviours for the Schedule constructor. There, therefore, should be two constructors. Possibly even two implementation classes.

locationNum and changeReason. These are currently weakly typed. They should probably have their own class. Following your question, the valid values depend upon the context. At the moment Schedule doesn't have any context. You could keep Schedule independent of context, in which case there are no database constraints on locationNum and changeReason. Going the other way, Schedule, locationNum and changeReason could be dependent on context, and the constructor should merely check that they are all in the same context. A package-private constructor (acting as a poor man's friend) may omit the checks.

A POJO implies a (well written) object and therefore behaviour. The question seems to be using it to mean "struct". Avoid structs.

Business layer seems to be misleading. It appears to be a business entity, it should have business behaviour. However, it may not contain business process (service).

Your application will always assume certain things about the database, including the (public) schema. If you remove constraints, then you may well break your application. This is the same as if you similarly changed code that is depended upon.

The common practice is to have anemic objects with procedural code. For most cases, this is not best practice.

Answer 5

If you really have "all those getters and setters" then you need to validate your class better than in the constructor. If the invariants of your class are such that all weekOfChange, dayOfChange and changeReason must be null or not all must be not null your setters are quickly going to put your class in an invalid state. Do you mean to have setters or do you mean your class to be immutable?

Before worrying about were validation should go maybe do an analysis of your class. Look at all its valid states and invariants for given states. Then you will understand whether your class should be mutable or immutable. Where you have mutually dependant covariants (like weekOfChange, dayOfChannge and changeReason it makes sense to package them into there own class and use composition in the Schedule class. This will put the "rules" about these things fields in a single place and simplify the Schedule.

The same with other collaborating fields (like cost and cost reason). Then Schedule is composed of to self validating classes. If both these are immutable and Schedule is immutable also you have your self a much easier domain.

So, to answer your question: it is better that a class defines its states and invariants and exposes only the minimum to its collaborators where practical. The responsibility for the internal state of Schedule should rest with Schedule and with just a little more design it can do with relative ease.

So you have

    class Schedule {
         private Change change;
         private Cost cost;
         private Location location;

        public Schedule(Location location, Cost cost, Change change) {
           this.change = change;
           this.cost = cost;
           this.location = location;
        }
}

And colloborators like

public class Change {
    private Date weekOfChange; //shoudln't these two fields be one, a date is a date after all??
    private Date dayOfChange; //shoudln't these two fields be one??
    private String changeReason; 

    public Change(Date weekOfChange Date dayOfChange, String changeReason) {
      this.weekOfChange = weekOfChange;
      ...etc.
    } 
}

Though I would strongly advise you protect your classes invariants with defensive copying of any mutable values that are passed in by client code.