3 Coding Principles Everyone Should Know
This post will describe 3 of the main principles I use when coding. These principles help me to write clean, readable and manageable code.
1. Get out quick (Short-circuit methods)
When writing methods, always be trying to return as soon as possible. Take the following example:
/** * Fetch the usernames in the specified group. If a username is provided, lookup just that single * {@link User}. * * @param group The name of the group within which the user belongs. * @param username The optional username with which to lookup the user. * @return A collection of {@link Users}'s. */ public Collection<User> getUsers(String group, String username) { Collection<User> users = new ArrayList<User>(); Collection<String> groups = getGroupNames(); if (group != null && groups.contains(group)) { if (username != null) { User user = userRepository.findByUsername(username); users.add(user); } else { users = userRepository.findByGroup(group); } } return users; }
There is no point instantiating the users and groups collections (or doing anything else for that matter) if no group parameter has been supplied. Take the following example:
public Collection<User> getUsers(String group, String username) { if (group == null) { return Collections.emptyList(); } Collection<User> users = new ArrayList<User>(); Collection<String> groups = getGroupNames(); if (groups.contains(group)) { if (username != null) { User user = userRepository.findByUsername(username); users.add(user); } else { users = userRepository.findByGroup(group); } } return users; }
Notice that I have added an if condition to check that the group parameter is not null. If it is null I will simply return an empty collection. You may instead want to throw an exception if this is not an expected state, or move this check up to the calling class to prevent the method ever getting called with a null group value. I will improve it further with the following example:
public Collection<User> getUsers(String group, String username) { if (group == null) { return Collections.emptyList(); } Collection<String> groups = getGroupNames(); if (!groups.contains(group)) { return Collections.emptyList(); } Collection<User> users = new ArrayList<User>(); if (username != null) { User user = userRepository.findByUsername(username); users.add(user); } else { users = userRepository.findByGroup(group); } return users; }
Notice that I have moved up and reversed the if condition that checks that the group name is a valid name that exists and can be used to lookup the users. This condition will now check is the name doesn’t exist and return an empty collection if it doesn’t. Notice, also, that by moving and reversing this boolean condition we have removed the nesting of the subsequent logic.
2. Sing Responsibility Principle (SRP)
As stated by Robert C. Martin, in the book Clean code: A handbook of Agile Software Craftmanship, a method should only be concerned with doing 1 thing. Sticking to this principle will make your code much easier to manage, read, and unit test. Take the following example:
public Collection<User> getUsers(String group) { if (group == null) { return Collections.emptyList(); } Collection<String> groups = getGroupNames(); if (!groups.contains(group)) { return Collections.emptyList(); } return userRepository.findByGroup(group); } public User getUser(String username) { if (username == null) { throw new InvalidStateException("Username must not be null"); } return userRepository.findByUsername(username); }
Notice that where previously we had 1 single method doing 2 things (Looking up a collection of Users based on the supplied group name, and also looking up a single User if a username was supplied), we have now separated this functionality out into 2 methods.
One method is specifically concerned with doing the lookup of users based on the supplied group name, and the new method is specifically concerned with looking up the single User based on the supplied username. Now both methods can be unit tested individually, and it is must easier to understand what each method is responsible for doing.
3. Sniff out those bad smells.
The following are a few things to watch out for whilst you are coding to help you identify when you may be starting to allow bad code design to creep in.
Too many nested conditions.
Try to keep you nested if conditions down to as little as possible. More than 3 may mean you could be short-circuiting, or the method is trying to do too much.
Too much setup.
If you are having to write many lines of code to setup a unit test method, this could be a sign that the method under test is trying to do too much. Could the method be split out, as per the Single Responsibility Principle, making for smaller, easier to test methods?
More tips to come in a later post…