Unless you are a very old-style procedural programmer, chances are you’ve grown up in an environment that promoted keeping the body of methods as short as possible. Nowadays more and more advocate that the Nirvana is to have methods of 1 line of code.

The rationale behind is the false assumption that shorter methods are enough to make a code base easier to maintain. Because the shortest useful method is 1 line, filling your programs of 1-line methods grants the highest level of readability.

Try to look at the following code:

public static boolean isNotCancelled(Concert concert) {
   return !concert.isStatusCancelled()
}

In this example isNotCancelled was introduced mainly to wrap in a single method the negation of concert.isStatusCancelled().

At first this may seem a good idea to improve the readability of the code using isNotCancelled() but it turns out it causes more problems than the benefits it gives when you start considering wider context.

The example here is very easy but I hope it’s already enough for you to see the problems this approach can introduce.

The creation of a new method hides the real implementation

Encapsulation is good, we can all easily agree about that. What’s wrong is to think about encapsulation as a way to hide who a certain responsibility belongs to.

In this code it’s really responsibility of the Concert to know if it’s cancelled or not. By adding that extra static method we are moving the focus from the Concert to its consumer. I mean, visually it seems that it’s the user that decides if a Concert is cancelled or not. When we choose names for classes like ConcertStatusHelper we are reinforcing even more this change of perspective.

In terms of pure readability it’s not immediately evident what being “non cancelled” means. I have to enter the static isNotCancelled() to know that’s simply the negation of being cancelled with is computed somehow somewhere else.

It’s not OOP

One of the base principle of OOP is cohesion. Scattering logic in thousand different classes breaks that cohesion. Even when the problem is simply where to put an apparently innocent helper method like this.

Acknowledge your objects the full respect they deserve and don’t lose them trying to force Functional-ish Programming in a language that was (is?) not designed for that.

Every new method adds a level of indirection that makes the code harder to understand

You added this method hoping to make your code look cleaner. And the single line of code calling that method probably does look cleaner. What’s becomes more complex to track is what’s exactly the logic your code is applying.

Imagine you have to document/understand the exact behaviour of a code like this

if( isNotCancelled(concert) ) reserveASeat()

If you are lucky you’ll have to enter and read the implementation of isNotCancelled() just to realize the real logic is in concert.isCancelled(). That’s a stack of 2 levels to understand.

That’s at least double the complexity of a code like the following that may looks less tidy and modern at first but that requires to deal with a stack of only 1 level

if( !concert.isCancelled() ) bookAnyTaxi()

Often 1-line methods end with a return, which is very difficult to debug in languages like Java

You have a bug that you are asked to solve. You want to add a breakpoint to your method but then you realize it’s a1-line return method.

Most if not all IDEs in the Java ecosystem won’t allow you to see what the return value is. Sure you can add a watcher or evaluate a line of code but what if your code alters the status of an object? What if that change is not idempotent and so you cannot call it everytime you want?

Sure, you could temporarily change that code to extract a local variable that’s returned a line later.

public static boolean isNotCancelled(Concert concert) {
   boolean notCancelled = return !concert.isStatusCancelled()
   return notCancelled;
}

But isn’t that a clear sign that the 1-line implementation was a bad idea? And what if that code cannot be modified because part of a JAR you imported in your project?

Sure, you could inspect the value in the caller but what if you are dealing with a chain of 5 or more 1-line methods all based on return? The only thing you can do is either do some mental computation or set your breakpoint many many levels away from the piece you want to test.

A method that contains 1 line of code is like documentation: potentially it gets out of date the 1st second after it’s written

1-line methods are considered nice because they allow you to assign a descriptive name that explains what a line of code is doing. Doesn’t it sound to you exactly what documentation does? And aren’t we all aware that documentation is famous to be too often out of date?

What if tomorrow I change my method like this:

public static boolean isNotCancelled(Concert concert) {
   return concert != null && !concert.isStatusCancelled()
}

Does the name of the method still hold true or it should be changed in something like defensiveIsNotCancelled()?

A class with 1k small methods is not rich, it’s complex to understand

When I’m dealing with a new class I often take the time to read what are all the methods available. When I have to modify it I read also the private ones.

What I usually do is to ask the IDE to provide a list of all the available methods. How easy it is to get lost with classes defined in 1k methods?

Similar issue if you are getting familiar with a new class by scrolling its source code.

When the number of options is too big it’s extremely easy to pick the wrong one.

And don’t feel safe just because you marked 99% of the methods as private. When you’ll have to modify that class the API you’ll have to deal with will contain the full 100% of methods.

When are 1-liners allowed?

At this point you may ask: are you suggesting to go back to methods 1000-lines long?

Absolutely not. There are many good cases where 1-line methods serve their purpose.

That’s usually the case of when we want to encapsulate a complex call with many parameters.

Concert readConcert(Long concertId) {
   restTemplate.getForObject(baseUrl + relativePath, Concert.class, concertId)
}

In the same way, we can extract the code to generate the URL for endpoint we want to consume to a one-liner:

private String getConcertEndpointURL() {
   return CONCERT_SERVICE_BASE_URL + CONCERTS_ENDPOINT_PATH;
}

When we want to read the code to make a network call, we can focus in the call itself (first method). When we see an error or want to inspect which url we are consuming, we jump into this new method, and can narrow or context to a URL generation process.

This might be for you an example of an unnecessary one-liner (not sure). But to me is a very good one liner since it helps the reader focus on just what it needs to read at a time. It’s also true that it would be a good one-liner if the class where this method is holds no more than 5 methods and perhaps ~20 lines of code (is just a client for an external service). So the convenience of one liners, as you mentioned above, also depends on the class we are writing. When we have 100 one liners, maybe is time to reconsider the amount of responsibilities that class holds.

Another good example is when you want to encapsulate a complex piece of logic:

boolean isAvailable(Integer seatNumber) {
   return !isCancelled() 
      && !bookedSeats.contains(seatNumber) 
      && allSeats.contains(seatNumber);
}

What I strongly encourage you is to forget about this purist approach that very few are brave enough to criticize openly and start focusing instead on finding the right trade off between readability, common sense, simplicity of the overall structure of your full code base and not only of the specific method you are working on.

Use 1-line methods only when you are sure they really add tangible value.