The following are just my guidlines for writing good code. The advise here may be scattered at best contradictary at worst. I write primarily java so I guess that this advise will just reflect my current feelings on java like code.
What I think changes from time to time.
Everything should have one purpose.
This is pretty important everything should have one purpose and only one purpose.
Write code with this in mind always naturally its tempting to think I’m talking about classes here but in reality this extends to much more than classes.
Methods
Methods are a prime example. I often see code that looks like this.
public Response execute(params) {
// iterate over the all
// validate each one.
// find one parameter that you need to do something special with.
// Call an upstream service.
// validate the response from the call.
// finish iterating.
// do a calcuation on the result.
// if a result contains a certain value return
// process the rest of the result
// return a response.
}
Now theres nothing wrong with any of those steps in themselves. However what you end up with is a tangled mess of operations that are extremely difficult to do any refactoring to and is also difficult to test. How do you validate that your delegating to the upstream service correctly from tests? The answer is you simply don’t.
Now the first thing you should so is seperate the block into seperate methods, making sure each method has a descriptive name. This instantly brings up the readablity.
However even with that approach its still difficult to test. You can either make the methods protected for the sake of the tests. Or more likely ask yourself is the class itself doing too much? Validating an object and doing business processing are two seperate concerns. Ideally you want the business logic in a completely seperate class without the confusion of anything that isn’t a buisness rule. The business classes are really what your application is about in the end of the day and must be clean.
Classes
I won’t go into classes too much. There are already umpteen different rules to remember with classes, and some old tricks such as fitting everything that they do into one index card. What a will suggest is that what the class does or its responsiblities should be commented at the top of the class.
example
package com.wesley_acheson;
// Responsible for calculating the tax
// Based on the country
//
// delegates to CustomerInformationService to find out information about the customer.
public class TaxCalculator {
private CustomerInformationService upstreamService;
public TaxCalculator(CustomerInformationService upstreamService) {
this.customerInformationService = upstreamService;
}
public InvoiceEntry calculateBatchTax(OrderBatch batch, Customer customer) {
//Do some logic here.
//Don't forget to split the method up.
}
// Note that even though this just delegates to another call
// Its really saying this is the part of the Customer service
// that is of intrest to me as a Tax Calculator.
private TaxRates getTaxRateForCustomer(Customer customer) {
return upstreamService.getTaxRates(customer);
}
}
Packages
Packages are the most abused concept that I see. Most times when I look through a code base people are splitting their packages as follows.
com.example.servlets com.example.services com.example.dao com.example.entity
The problem with this is manyfold.
- Firstly try to make some of your internal methods have package level protection. This always breaks as it isn’t available.
- Secondly its hard to trace the flow of information arround so many different packages.
- Generating a diagram of a packages uses is difficult
The solution is simple. Keep objects that need to refer to each other to the same package. I’d much rather see people suffexing their classes with what the class is rather than split by package on arbitary lines.
The amount of packages you will need tends to vary with the complexity of your application. Try starting with one or two base packages first. Then as complexity grows move objects that aren’t related out.
Now this isn’t enforced by the java compiler but I like to see classes that are used alot in parent packages of the packages that call them. Thats personal taste however and if you perfer to put your util classes in a package called util then go for it.
One place that a package for all the objects may be a good idea is if following domain driven design then by all means put your domain objects in one package. I warned you my advise may be contradictary. The simple reason for that is if you have a well thought out domain model its probably better to see the relationships with that then splitting on other criteria.
Note I’d probably keep TaxRates and TaxCalculator In the same package. If I had a need for them. I’d also keep TaxDao and TaxService in the same place.
Duplication
It should be the aim of every programmer to keep code duplication to a minimum at all points. Copy paste is evil and you will get tripped up by it sooner rather than later.
The problem with copy and paste is its often the quickest way to achieve the desired result. However If you find a bug in one copy of a method and think there may be 50 derivatives what are you going to do?
This is getting long and windy I’m getting tired. I guess I should finish or cleanup on another day. Love to hear from you all what your opinions are.
