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:
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.
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