December 29, 2012

CodeSense: Implementing Methods

As I mentioned in an earlier CodeSense post, methods describe the functionality that your class provides. In this article I'll explain how to implement methods in a way that increases the readability and maintainability of your code. This article is by no means an exhaustive treatment of best practices, rather it is an overview of high-impact changes you can make to your coding practice right now to significantly improve your work product.

General Principles to Follow When Implementing Methods in Your Classes

What's In a Method Name?

If you haven't already read Smart Naming Practices, go ahead and read it now. Don't worry, I'll wait...

The Single Responsibility Principle and Atomicity

Methods should do one thing only, whatever that one thing happens to be. I've written and reviewed some monstrous methods that performed several actions within the method body, and I can tell you from experience that they were awfully difficult to test and even more difficult to maintain.

Additionally, methods that do multiple things are rarely (if ever) reusable for their constituent functionality. Let's say I have an application that accepts user registration. I've written my registration method such that it inserts the user's details in a database and sends the user a confirmation email. After several months I start to receive institutional accounts that want to add users but do not want the confirmation email to be sent. What do I do now? 
  • Create a new method that only inserts users into the database? Great, now I have two methods I'll need to maintain whenever I have to change something related to adding users to the database.
  • Add a flag to the method's parameters to indicate whether the confirmation email should be sent, and wrap the code that sends the email confirmation in a giant if-statement? Ok, that might work in this sort of binary scenario, but it does have a certain smell if you ask me.
Neither of these two options is very good. It would be better instead to separate the database function from the email function, and use any one of a number of patterns to implement the use case that requires both functions. 

The simplest thing to do would be to create three methods - one that adds a user to the database, another that sends a confirmation email, and then a third that calls each of these methods in succession. Yes, if we do that we'll still have a method that does more than one thing, but at the very least we'll have isolated the two separate actions being performed within that method so that we can reuse one or the other in the future.

The overarching message here is that you should see your methods as building blocks of atomic functionality that can be used separately or together, rather than seeing your methods as do-it-all buckets of code that are written and rewritten over and over again for each use case.

Overloading and the YAGNI Principle

Overloading a method means creating several different implementations of the same method, each with a different signature. Let me tell you a story about overloading. Early on in my career I was asked to create a method that would serialize the containing object as XML for storage and transfer. Perhaps I wanted to show someone that I knew a thing or two about OOP, or maybe I thought I was preparing for the future. Either way, I created not only a method to serialize the object to XML, but several overloads to serialize it to JSON, YAML, url-encoded string, you name it... none of which was necessary because none would ever be used.

It's often tempting to try to anticipate the future by creating methods (or overloading methods) that aren't needed right now, but might be needed in the future. I find this temptation to be particularly strong when it comes to overloading methods. But, chances are, You Ain't Gonna Need It. Create only the methods that you need right now. Implementing only the methods you need at any given time prevents you from writing code unnecessarily that you will have to maintain in the future even if it is not in use.

Avoid External Dependencies

I'm sure to most seasoned developers this sounds like a pie in the sky notion, but I don't think that methods have any business relying on any object or data that isn't explicitly passed in to the method as a parameter. There are two problems inherent to relying upon external state in a method: first, it creates a tight coupling between your method's class and the external state's class. The second problem follows from the first: such a tight coupling makes it difficult to reuse your method's class.

So what's an external dependency? An external dependency is any object or data that is referenced within the body of your method that is not passed into the method as a parameter. The most obvious examples of this type of dependency are session state, application state, configuration settings, and static dependencies like static. However, even the implementing class's state can be considered an external dependency depending on how strictly you interpret the definition.

Avoid Side Effects

In theory, methods are meant to perform transformations on data and then return the result of those transformations. Any behavior that falls outside of that description is considered a side effect. In more practical terms, a side effect is any change in state that occurs within your method body that is not expected or explicitly defined. 

We expect that mutator methods will change state - that's actually their entire purpose, so it's not really a side effect. However, it is generally a side effect if any non-mutator changes state either within the implementing class or - gasp - outside of the implementing class. It is a good practice to avoid creating side effects by avoiding code that explicitly or implicitly changes the state of the implementing class when such a change in state is not to be expected as a result of calling that that method. In practice this means that any method you write that returns a value should not update fields in your class or outside of your class (like session, cache, etc). 

Save Conversions for the Method's Client

It is the responsibility of the client to perform data conversions, both for input parameters and return values. It's poor form to return an int as a string, accept a string when you are performing operations on a double, etc.

Don't Eat Exceptions

There's nothing more infuriating to me than to debug code and find that there's an empty catch block swallowing a thrown exception. If you've put code in a try block you've done so because you think your code might throw an exception. If that's the case, you should either handle the exception or re-throw it. In general if you are thinking of eating an exception you should re-throw it instead. That brings me to my next point...

Fail Loudly 

Code that fails but does not throw an exception is poorly written in my opinion. Some programmers like to return status objects (or return boolean and pass a status object as an out parameter) rather than throw exceptions, but I just don't see the point. Why should a method continue execution if, for example, it has been passed an invalid parameter? The most common argument is usually that the method should recover if it can, but I don't think it should be the method's job to handle recovery... that's the method caller's job. So, long story short, if your method's execution fails it should throw an exception!

Accessor and Mutator Methods

Accessors and Mutators are methods that retrieve and modify the state of your class, respectively. While some languages can automatically implement these methods for you, many do not. Accessor methods should be named getFoo (or GetFoo, if Pascal case is preferred) and mutator methods should be named setFoo, where foo is the name of the backing field. It is permissible to name boolean accessor methods isBar, hasBaz, etc if that is preferred.

It is absolutely critical that your accessor and mutator methods do not modify state unexpectedly. In other words, your setFoo method should set the value of the foo field, and do nothing more to modify the object's state.

Collection Methods

It's generally not a good idea to provide accessor and mutator methods for collections that belong to your class. Rather, it is better to provide methods to add, remove, and retrieve specific items from collections.

Static Methods

A method should rightly be static if it can be applied to all instances of the class regardless of the internal state of the class. In other words, if you write a method that does not call accessors or reference backing fields, nor call mutators or update backing fields, then it can (and should) be a static method.


In most object-oriented languages you can specify whether your methods are public, protected, or private. Public methods can be accessed from any code within or outside of your class. Protected methods can be accessed from within your class or any of its subclasses. Private methods can only be accessed from within your class. My general approach with accessibility is to make all my methods private, and then increase accessibility to protected or public as needed. This has always worked well for me, so I think it's a good approach. Obviously if you're creating a method in a class to meet an external requirement then it's okay to set its accessibility level to public initially.

Thanks for checking out this article on methods, which is part of a new series called CodeSense. CodeSense is about making code easier to read and write by developing common sense conventions that can be applied immediately in nearly any programming language. Please contact me at brian (at) brian-driscoll (dot) com if you have questions, comments, or requests for future CodeSense topics.


Paul C Smith said...

Great article; I agree with most of these recommendations and choices. One exception is making methods static if they don't access instance state. Using instance methods does more than just give one access to instance fields; it also lets the method belong to an interface, making the class more amenable to dependency injection, service locators and the like. I've personally gotten burned more times by making a method static than by making it an instance member.

Anthony said...

I agree whole-heartedly with failing loudly. Nothing more infuriating then wasting days trying to find whats wrong in a complicated section of code only to find out some exception was being throttled silently because the testers were bringing it up as an issue. No! the exception is not the problem, whatever is causing the exception is!