Monday 24 May 2010

Empowering your entities

The first thing I learnt when I started with Object-Oriented Programming was that an object should have state and behaviour. And indeed, that's how it was back then. However, since J2EE (Entity Beans) and other ORM tools like Hibernate, iBATIS, TopLink, JPA, etc, everything changed. We were so focused in mapping tables to objects, configure the relationships, lazy loading, eager fetching, primary keys and everything else that you can map and configure on the ORM tools that we simply forgot that those entities could and should also have some behaviour.

In general, for applications that use a database, the entities are the key focus of the application. They are the main components of an application's core domain and their names and methods should represent, and also help to define, a common language used by all members of the team and not just developers (See Ubiquitous Language - Domain Driven Design). 
 
Let's see a few situations where your entities could be empowered.

1. Default values / initialisation

Let's have a look at a fairly simple piece of code and see how many problems it may cause:

Listing 1.1
// Somewhere in the code 
DiscussionGroup dicussionGroup = new DiscussionGroup();
discussionGroup.setCreationDate(new Date());
discussionGroup.setLastUpdate(new Date()); 
discussionGroup.setAllowAnonymousPosts(false);

// Somewhere else in the code
if (discussionGroup.getAccess().equals(Access.PUBLIC) {
   this.displayDiscussionGroupInfo(discussionGroup);
};

Members members = discussionGroup.getMembers();
for (Member member : members) {
     // Do something
}

This code above has the following problems:
  • creationDate and lastUpdate setters being called right after creation. Let's assume both attributes are mandatory on the database. Wherever in the code a DiscussionGroup is created, we need to remember to set them both. In Failing to do so, we will get an error when persisting the entity.
  • The check to verify if the discussion group is public may throw a NullPointerException if the attribute is not initialised.
  • Like the access check, the iteration over the discussion group members can also fail if the list of members is not initialised.
  • The calling code needs to know about the internals of the entity and cater for that, like setting values to mandatory fields and also do null checks for some attributes.

To solve the problems, just initialise all the attributes you can, I mean, the ones where a default value would make sense. Always avoid having getters returning null.

Listing 1.2
// DiscussionGroup.class
public class DiscussionGroup {
    private Date creationDate;
    private Date lastUpdate;
    private boolean allowAnonymousPosts;
    private Access access;
    private List<Member> members;
    
    public DiscussionGroup() {
        this.creationDate = new Date();
        this.lastUpdate = new Date();
        this.allowAnonymousPosts = false;
        this.access = Access.PRIVATE;
        this.members = new ArrayList<Member>();    
    }
}

2. Operation that just involves an entity's attributes.

Look at the code below.

Listing 2.1
// OrderService.class
if (order.getTotal() > 0 
        && order.getOrderItems().size() > 0
        && order.getAuthorizationDate() != null 
        && order.getClient() != null) {
    this.processOrder(order);
}    

The if statement checks attributes from the order object so that it can decide if the order can be processed or not. Note that this logic is outside the order object, making the two classes tightly coupled and making a poor use of encapsulation. In this case, we could re-factor this code like that:

Listing 2.2
// Order.class
public boolean isReadyForProcessing() {
    this.getTotal() > 0 
        && this.getOrderItems().size() > 0
        && this.getAuthorizationDate() != null 
        && this.getClient() != null);
}      

// OrderService.class - Somewhere in the code
if (order.isReadyForProcessing() {
    this.processOrder(order);
}

This re-factored code makes the code more expressive, easy to read and easy to test. This also makes the code to better represent the business rules.


3. Children manipulation

Look at the listings 3.1 and 3.2.

Listing 3.1
// Add an entry to a diary.
Diary diary = new Diary();
Entry entry = new Entry();
entry.setDate(new Date()); 
entry.setText("Today I went ... ");
diary.getEntries().add(entry);

Listing 3.2
// Remove an entry from a diary.
ListIterator<Entry> entriesIterator = 
        diary.getEntries().listIterator();
Entry entry;
while (entriesIterator.hasNext()) {
    entry = entriesIterator().next();
    if (entry.getDate().equals(someDateVar)) {
        entriesIterator.remove();
    }
}

The main problem with Listing 3.1 and Listing 3.2 is that the parent class Diary is exposing its internals, the entries. Once again, this is a major encapsulation breach. As a principle, the parent should never let a stranger manipulates its children without its consent. Besides all the code duplication that this may cause, in case that I want to add/delete entry in different parts of the application (for some reason), this also makes the code to be very fragile where the parent may break since it was not notified that someone changed its children.

This could be easily fixed if we add this logic to the parent class.

Listing 3.3
// Diary.class
public Entry addEntry(Date date, String text) {
    Entry entry = new Entry();
    entry.setDate(date);
    entry.setText(text);
    this.entries.add(entry);
    return entry;
}

public Entry deleteEntry(Date date) {
    ListIterator<entry> entriesIterator = 
            this.getEntries().listIterator();
    Entry entry;
    while (entriesIterator.hasNext()) {
        entry = entriesIterator().next();
        if (entry.getDate().equals(someDateVar) {
            entriesIterator.remove();
            break;
        }
    }        
    return entry;
}

The calling code would be like:

Listing 3.4
// Add an entry to a diary.
Diary diary = new Diary();
Date date = new Date();
diary.addEntry(date, "Today I went ... ");

// Remove an entry from a diary.
diary.deleteEntry(date);

Conclusion
Entities don't need to be just dumb classes with state and no behaviour. They should contain business logic that is related to their attributes and children. This will promote encapsulation, reduce code duplication, make your code more expressive and easy to read and also much easier to test.

Sources:
http://domaindrivendesign.org/
Domain Driven Design - by Eric Evans
Clean Code - by Robert C. Martin

2 comments:

mash said...

Hi Sandro, Great post. I have one suggestion. Instead of creating the add and delete entry methods I would change the getEntries method to return an Iterator with a custom implementation (possibly implemented as a private member class). This would allow the client to iterate over the entries while maintaining control over their manipulation. I am assuming that iteration over the entries is desired.

Sandro Mancuso said...

Hey Mash. For a normal object, that would be the best approach indeed, never exposing the internal collection. However I had problems doing that for entities. The reason was that getEntries() was mapped in Hibernate/JPA and I could not "hide" this collection. I also had problems in returning an immutable collection.

Post a Comment