How thin is too thin?

I recently had a conversation with a coworker about where the logic belonged in a piece of code.  I felt that his dao* (data access object) layer had too much logic.  I suggested that the objects that were communicating with the database should only have the queries needed to get the data from the database, and any business logic should be in a different layer (likely a service layer). My colleague said that he had done that before and he thought that made the dao layer too thin.

I let the comment slide, and let the code slide (though I am regretting that decision).  But, that got me to wondering if there is such a thing as a layer being “too thin”.

tomatoes
This, by the way, is probably too thin.

The only example I can think of where I might say that a layer was too thin was if it were just a wrapper around another object and it wasn’t really serving any purpose, it just called through to the object it wraps.  However, if the wrapper was serving a purpose (like wrapping a utility object that was all static methods so we can mock it, or wrapping a third party library that we want to encapsulate to make it easy to change out), then I wouldn’t have an issue.

In my mind, I try to think of each layer as a different level of abstraction.  Our controllers have the highest level of abstraction.  Reading the code there helps you understand the high level tasks we need to do (like, get some data, set some values and return some data).

The next level down is the service layer.  This contains business logic  It would contain the code that sets values, or does some calculations that we need to return to the user through the controller.  This layer is more detailed, but will often use other classes to assist.

The last level is the dao layer.  This is where we are getting our data, either from a database or file, or magic 8-ball.  This has nitty-gritty details of how to get the data, but contains NO logic for converting the data in any way.  It can contain queries that allow us to bring back only the data we are interested in.  Those queries can do things for us like sorting our data, etc.  But, the dao layer is simply in charge of executing the queries and returning the data to the service layer.

Do you have any examples where you have seen a layer and thought it was too thin?

*A data access object is any object that is used for accessing data, such as code to talk to a database, or read from a file.  It is a commonly used Design Pattern.  Look forward to a future post about this.
Advertisements

Unit Testing – Part 1

I’m fairly new to writing automated unit tests. I’ve read about it and I’ve tried it out and we even have a handful of java classes (10 to 20) that have full coverage from unit tests. But now it looks like we might be changing our view of testing and diving in head first.
So here are some things that I’ve learned that I didn’t really pick-up on when reading the books about JUnit.

Step 1 – Write Testable Code
Something that I missed when reading books about unit testing is that you have to change the way you write the code in the first place. This is generally a good thing, but something that is difficult to do across an entire development team.
Long methods – are very difficult to test – We have some methods that are over 1000 lines, it’s not easy (maybe not even possible) to test a method that is this big.

Dependency Injection – methods that create their own database connections, or create any new objects directly are very difficult to test well. For legacy code, I’ve found that a good way to handle this is updating the method to take these objects as parameters, and then creating a new method with the old signature that simply creates the objects and passes them in. This allows you to create mock objects for database connections, etc. which lets you test all the code in the method without needing a database to connect to.
ex:
public User buildUser() {
Connection conn = getConnection();
return buildUser(conn);
}
public User buildUser(Connection conn) {
... existing/old code to read user object from databse ...
return user;
}

References to Static Methods – Static methods are easy to test. Code that calls static methods are VERY hard to test, because you can’t mock/stub out the static methods. So you end up having to test all the code in the static method along with the code in the method you actually want to test. I don’t think small methods that take few or no parameters from well tested libraries are much of an issue. For example, if your code calls Math.random(), it’s still fairly easy to test. But if you have a lot of “utility” methods that take large objects as parameters or that do complicated logic it’s difficult to mock-up the test data to get those static methods to return all the different scenarios that allow you to run through all the code in the method under test.

If developers are writing code that is difficult to test, they will hate writing unit tests. It is time consuming, the tests will break often from small changes in the code, and code coverage will be low. They need to change the way they write the code in the first place before you can be successful in implementing unit tests.

In general, testable code is better quality code anyway. Having short methods that “Do one thing”, and creating methods that use Object Oriented Design (ie: not static) are easier to maintain, easier to read, and much nicer to work with. This is a side benefit of implementing unit tests.

I’ll continue with my lessons learned in later posts. Something I’m still not sure how we are going to handle is all the existing legacy code that doesn’t fall into the category of “testable” or at least not “easily testable”.

Java – Static Methods

I currently work in a code base where static “utility” methods are used fairly often.  This probably stems from the legacy of the code base.  Back when the company was just getting started, there were only a couple of developers (including myself).  While we all knew how to write Java code (syntax), none of us knew how to write good Java code (Object Oriented, encapsulated objects, etc.).  So we wrote a bunch of static utility methods trying to keep the code clean.

Ten years later, I’m still with the company (as are 2 of the other 3 original developers) and we are still working in the same code base.  The company has grown steadily and there has never been available resources to take on a whole system re-write.

The original code from 10 years ago isn’t the only problem.  The development team has grown significantly over the years as well.  Most of the team consists of developers that have very little experience in Object Oriented Java before joining our team.  So they tend to use the existing code as an example of how to write new code, which means more static utility methods and the cycle continues.

So what’s wrong with Static Methods?

If used in the right context, there is nothing wrong with them.  But using them to often or in the wrong places has several issues.

Automated Unit Testing

Static methods make implementing automated unit testing very difficult.  There is no easy way to stub out the methods.  There isn’t an interface to program to, and you can’t use polymorphism to override the static method to return a specific result for testing.  So when writing unit tests on code that uses static methods, you end up testing the not only the code in the method you are trying to test, but the code in the static method(s) as well.  This is often the biggest reason given for not using static methods.  The company I work for doesn’t use automated unit testing (I’ll post about that another day), so this doesn’t work as a reason not to use them for us.

Object Oriented

Static methods do not extend any other class or methods and cannot be extended.  So the code in them can not be reused – unless it does exactly what you need it to do already. Static methods are not Object Oriented in anyway. I’m not saying they can’t be utilized in OO code, just that they themselves are not.

Readability

Static Methods alone don’t make for hard to read code, If they are over used or the class or method name is long it can certainly detract from readability:

// Harder to read
User user = new User(StringUtility.removeSpecialCharacters(firstname), StringUtility.removeSpecialCharacters(lastname));

// Easier to read
User user = new User(firstname.removeSpecialCharacters(), lastname.removeSpecialCharacters());

Passing the same object to multiple static methods to have it just return the same object (with modified data/state) quickly becomes difficult and tedious to read.

// Harder to read
order = CartUtility.calculateTaxes(order, billingState);
order = CartUtility.calculateOrderTotal(order)
print("OrderTotal:" + order.getOrderTotal());

// Much easier to read
order.calculateTaxes(); //better yet, make this part of calculateOrderTotal()
print(“OrderTotal:” + order.calculateOrderTotal());

So when SHOULD I use a static method?
You should use a static method if (and only if) ALL the following are true:

You are writing utility class and they are not supposed to be changed.
The method is not using any instance variable.
You aren’t passing in a parameter and getting the same object returned. Generally this means the method should be put in the object you are passing/returning.
It just doesn’t make sense to create them as instance methods for some reason.
You are sure that the definition of the method will never be changed or overridden.

What are your thoughts? Leave a comment below and let us know