This is a continuation of the post devoted to some code smells that was vigorously commented in favour of ugly, anti-OO code design :). Thanks God, I also got an interesting question regarding this problem.

Seems to me, that this topic should be discussed one more time.

Introduction

Let me remind you what the problem is.

The original code (in some class, other than Order class) looks like this:

  String custID = order.getCustomerID();
  String itemID = order.getItemID();
  int qty = order.getQty();
  double price = order.getPrice();
  if (custID != null && itemID != null
          && !custID.equals("")
          && !itemID.equals("") && qty > 0
          && price > 0.0) {
     return "ORD1234";
  }

This is definitely not OO, and against the famous Tell don't ask principle.
The code is procedural, which means, that other objects take values from Order, examine them, and make judgements based on thems. All getters are now part of the Order API, which makes any changes of Order class troublesome.

A more OO approach is to hide the validation logic inside Order class:

  if (order.isValid())
     return "ORD1234";
  }

And the Question Is ...

Surprisingly I got few comments in favour of "getters" approach ((btw. all comments so far were anonymous).

One of the readers asked an interesting question:

That is a good question. What if for different clients using Order class "a valid order" means different things ? Let us see.

I think that there are two possible situations:

1. you know what kinds of validation will be required while writing Order class;
2. you expect that various kinds of validation will be required, but you don't know what kinds exactly.

Let us take a closer look at each of them.

List of Required Validations is Known

In such (unrealistic) case, you simply add as many validation methods as required. You end up with something like this:

public class Order {
  // a lot of fields and methods that we don't care about at the moment
  ....

  // here comes validation methods
  public boolean isValidA {
    ...
  }
  
  public boolean isValidB {
    ...
  }

  ... /// more of validation methods
}

Of course, you would choose more meaningful methods names!

This case is unrealistic, let us move on.

Allow Client Do Whatever Validation They Wish But Protect Your Code

This time we want to allow clients to provide their own validation methods and at the same time, we refuse them access to the internal structure of our Order class. Is it possible? Yes.

First, create an interface for order validators:

public interface OrderValidator {
  void setCustomerId(String customerId);
  void setItemId(String itemId);
  
  // more similar methods here
  
  // and now the validation method
  boolean validate();
}

Please notice, that the setters do not have to inject all the fields of Order class. They should allow validators to acquire all information that is important for validation - and nothing more.

The Order class should provide a method, that accepts implementations of OrderValidator interface:

public class Order {

  // a lot of fields and methods that we don't care about at the moment
  ....

  public boolean isValid(OrderValidator validator) {
    validator.setCustomerId(customerId);
    validator.setItemId(itemId);
    // more setting of values here
    ...
    // at this point the validator has all the data
    // and can do its job
    return validator.validate();
  }

}

Now the clients can come up with whatever fancy implementation of OrderValidator they wish, and you are even able to perform some manipulations on the the internals of Order class without breaking the clients. Sweet.

And that would be it. The question is answered. ...but I feel that you would like to know why do I expect you to trouble yourself with additional interfaces and classes, wouldn't you ? Keep on reading then.

But What About Tests ?

Let us say we have another class - OrderService - that rejects orders that do not validate, and saves only valid ones - sth like this:

public void saveOrder(Order order) {
  if (// TODO check that order is valid) {
    orderDAO.saveOrder();
  } else {
    throw new SomeMeaningfulBusinessException(...);
  }
}

Now, you want to test the behaviour of this method of OrderService. In particular you want to know that:

• saveOrder method of OrderDAO is executed only on valid order;
• in case of order that is not valid, a proper exception is thrown.

Please notice that for the sake of simplicity a pseudocode will be used in the examples of tests.

Testing - OO approach

The tests are straightforward:

public void validOrderIsSaved() {
  expect(mockOrder.isValid()).andReturn(true);
  expect(mockOrderDAO.saveOrder(order))
  replay(mockOrder,mockOrderDAO);
  orderService.saveOrder(mockOrder);
  verify(mockOrder, mockOrderDAO);
}

@Test(ExpectedExceptions = SomeMeaningfulBusinessException.class)
public void forNotValidOrderAnExceptionIsThrownAndOrderIsNotSaved() {
  expect(mockOrder.isValid()).andReturn(false);
  replay(mockOrder, mockOrderDAO);
  // probably try/catch needed here, so the verification 
  // of mockOrderDAO is executed, but let us skip it
  orderService.saveOrder(mockOrder);
  verify(mockOrder, mockOrderDAO);
}

Testing - "getters" approach

In case of using "getter" approach you need to know a lot about the validation of an order - you need to know the algorithm of validation, so you can return the proper values to receive true and false respectively.

public void validOrderIsSaved() {
  // doesn't matter if I use a mock or a real Order object
  order.setCustomerId("123");
  order.setItemID("456");
  order.setQty(111);
  order.setPrice(2.3);
  expect(mockOrderDAO.saveOrder(order))
  replay(mockOrderDAO);
  orderService.saveOrder(order);
  verify(mockOrderDAO);
}

@Test(ExpectedExceptions = SomeMeaningfulBusinessException.class)
public void forNotValidOrderAnExceptionIsThrownAndOrderIsNotSaved() {
  // doesn't matter if I use a mock or a real Order object
  order.setCustomerId("123");
  order.setItemID("456");
  order.setQty(111);
  order.setPrice(-7.8);
  replay(mockOrderDAO);
  // probably try/catch needed here, so the verification 
  // of mockOrderDAO is executed, but let us skip it
  orderService.saveOrder(order);
  verify(mockOrderDAO);
}

The tests are a little bit longer, but this is not the problem here. What will really hit you hard during further development is that many of your classes are tightly coupled with Order class and its validation logic.

Conclusion

The code speaks for itself, but let me summarise what we have just learned.

The conclusions are rather obvious:

• If you think in long term, there is no way you will use get/set approach.
• If you hope for a short living relationship with your code, you might be tempted to use the "get/set" approach. As your relationship lasts longer (than expected) you will regret your decision.

BTW. Have you noticed, that you will never get into "get/set" horror design if you really, honestly, passionately test your code ? (Especially if you do it test-first).

The Dark Side is Very Strong

P.S. Picture of Yoda was taken from druffmix.com site.