Clean Code

Practical Clean Code 1: Variables Methods Classes

Practical guide for applying and evolving clean code, design patterns to domain driven. This series will highlight each of the steps and then connect all of them into the big picture so everything can make more sense. This article being the first, focusing on clean code at class level and below.

Clean Code

Overview

By no means this article is comprehensive. This article highlights items which in my experience would help start off. Readers should expand further by reading books on the subject.

Clean code here, means that the code has qualities that can make developers / programmers understand and change it easily such as (but not limited to):

• Conveys clear intention
• Easy to understand (the flow, logic, at every abstraction level)
• Easily update-able and make changes (changing one thing doesn’t cause a chain reaction, )
• Testable

Having clean code means you easily update your code to match you current understanding, design and model of the system. To safe guard your refactoring efforts, tests must be in place.

Tests

One of the things that enables you to convey clear intention is to ensure that the code models the problem domain closely and up to date as it the design becomes clearer. This will ensure the code is self documented and the basic assumptions of the code are as expected. What better ways than to test your “assumptions” via tests :P.

You can choose between unit tests, integration tests (test containers, embedded db, etc.) within your project by deciding how much value / test coverage vs the effort. Building an in house product would be ideal to have test at all levels (unit, integration and system) but for projects that’s one off with tight dateline, it’s better to choose something high value vs effort IE. integration test (though trade off tests run much slower).

More details on tests will be shared on separate article:), but at the very least you can do in the beginning, is to separate object construction and its usage.

Construction vs Usage

To improve test-ability and re-usability, we need to separate object construction and usage. Without separation, both construction and usage will be intertwined at the same place / class. Example:

public class SomeClass {
  public void someMethod() {
     final Object something = new Object(); // construction
     object.something(); // usage
  }
}

The “object” should be instantiated somewhere else and not within the class that is using it. Perhaps it can be constructed in the main class, factory or if you are using DI framework in it’s registry mechanism (for Spring case would be as the bean). Then the class that’s using the object should have a mechanism to pass the constructed object either via constructor, setter or in Spring case (if you choose to) reflection.

Continuing on from the example above, the object can be passed to the SomeClass via constructor.

public class SomeClass {
private final Object thatObjectYouWantToUse;
  public SomeClass(final Object thatObjectThing) {
    this.thatObjectYouWantToUse = thatObjectThing;
  }
public void someMethod() {
     final Object something = new Object(); // construction
     thatObjectYouWantToUse.something();
  }
}

This gives few benefit:

1. Test-ability by allowing you to substitute the “Object ”with something else (mocking it for example) for testing via dependency injection.
2. Re-usability, by allowing to reuse existing implementation with different behavior (IE: you have a class that uses another object, you can pass a different object here that does something different)
3. Flexibility, by giving you path ways for expand-ability, 1 instance, through multiple implementation of the same concrete class via interface (design patterns, hint* strategy pattern for example hint*, more details on this later)

There’re more subjects like how to make test less fragile, clean tests, test smells, patterns, but we are not get into that for now. The points above would be somewhat enough to get you started.

Just that, I can’t stress enough how important test is in making your code clean.

We are going to use a web gallery application for back-end for some of the examples to illustrate the points below. For other cases I would use generic naming scheme to illustrate the intention of applying cleaner implementation.

Variable

Be Generous in Giving the Name

Don’t be stingy with the number of character. Try to give a name that reveal the intention of storing the data in the variable.

You have a URI for un-edited and un-resized image.

private final String uri;// not this
private final String image;// not this
private final String rawImageUri; // use this

Use variable to add clarity

Let’s say you have a situation where you have the thumbnails URI.

1. Then this URI will be prefixed with the CDN server.
2. This CDN server will depend on some condition within Thumbnail.
3. If thumbnail has has large sized url link (I will use simple boolean flag to illustrate), use the default CDN server.
4. If not append the CDN based on the region passed.
5. Finally convert it to another type, response for example.

If you are > java 8, you might have something like the following:

Gist 1 Variable Clarity

You can assign in between to a variable before proceeding to the next step. This variable you can use a name that illustrate the intention. Line 6, using the variable we are showing the steps are to build a list of “thumbnailWithCdn”. Line 12, we are building the “responseData ”for downstream consumption.

The same idea should also be used to illustrate methods.

Method

Your Intent in Action

Method name would be doing something with the intention of doing it.

getSomething()
resizeAndCompressImage()
prefixImageWithCorrectCdn();
removeItemIfNeeded() // the item is removed if the condition is met.

Method name should be a concise description on the steps within the method, and the end result should be the reader should have a general idea what the method is doing just by glancing the method’s name.

The need to comment is the need to rename

If you feel you need to write a comment to clarify the method’s intention then it’s a sign you should rename your method to something else. Comments should be reserved for whys and context of the method implementation and should not be used to clarify your intention.

Long Method Name (too long)

Might be an indicator you need to break the method further. Try to think at which abstraction level you should break and see it makes sense.

checkValidityAndBuildUrlIfNecessary() // you can break to below
final Map<String, Validity> objectIdValidityMapping = checkValidity();
final List<Object> objectWithUrl = buildUrlIfNecessaryWithValidity(objectIdValidityMapping);

Choose Proper Abstraction Level

For example you have the following steps you need to do.

1. Get some data from the database
2. Based on the data check on an external system (REST endpoint perhaps) to verify something (legit or not, authorize or not, anything)
3. Then after that, based on verified items you need to access another external system to get additional data (REST perhaps)
4. Finally merge both data and return.

Below you can see 3 methods, first without any abstraction whatsoever. Second with mixed abstraction (one of the step is extracted as method). Finally, last one, all encapsulated in methods at proper abstraction level.

SomeData being just an example data structure for the initial call, SomeData2 is used as ab example to represent the data from external system. You can imagine SomeData could be your profiles, accounts, books some entity as basis. SomeData2 could be your data extension, address, authors, supplement accounts etc.

Do One Thing in a method (Same Abstraction Level)

After understanding a bit on proper abstraction, next, is to ensure that each method do one thing at a given abstraction level.

Looking above, “getVerifiedSomeData()” method line 82, it’s consist of few steps,

1. Getting the raw data from database
2. Converting the SomeData to just the ids for next step usage.
3. Converting map of the id and the verification (just a boolean)
4. Returning verified SomeData, meaning filtering our the false verification in step 3.

All the above steps can be abstracted at 1 level in the method “getVerifiedSomeData()”.

If… else… then?

Having too many if elses (and nested ones) increases the cyclomatic complexity (simpler term, complexity of the code :P), https://en.wiktionary.org/wiki/cyclomatic. Try to cut it down by making the conditions simpler, or thinking a simpler logic. If not, encapsulate each branch in a method.

If the if then elses are part of checking mechanism for null or empty, you can reduce it by:

1. Using optional.orElse return default value (java > 8 ), https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html#orElse-T- (line 4)
2. Apache’s ObjectUtils comes in handy to minimize if else, https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/ObjectUtils.html#defaultIfNull-T-T-. Will use the default if it’s null. (line 6)
3. You could also use switch by identifying the proper flag for the switch to trigger. (quite straightforward for this one)
4. Encapsulate the decision making in another method. (line 8)

The above just to check nulls and empty value. You can replace this with something that fits your case. If that can’t be done, there’s a cleaner way to go, the chain of responsibility for example :). We will get to there later.

Consistency

If you are using a variable call image and a method doing something on it should also refer to the image in the method name.

private final String imageUri;
private void selectCdnForImageUri () {..};
// NOT
private void selectCdnForPictureUri() {..};

Another point for consistencies, is the term use in discussions either business or technical in nature. That term should be the same when discussing with Analyst, Product Owners, business stakeholders, developers, architects and other technical stakeholders. If there’s a translation or mapping required from business to technical model or vice versa, then there’s a gap that has a high chance of causing miss-communication and confusion.

Consistencies, consistencies, consistencies…

Class

Naming

Should use noun or noun phrase that concisely describe the concept. This is important because from the name, if it’s define correctly, it will help you in making the class cohesive. In software engineering we strive in maximizing cohesiveness and minimizing coupling.

Cohesiveness

If you’re able to name the concept concisely, then the class should only do what it suppose to do within the boundary of its concept definition. This basically “centralized” the concern of the concept and ensure if changes are needed (at least for that concern),only 1 location are needed to be modified.

From understanding perspective, it’s easier to get the mental picture of what’s going on. Having mix bag of things in the class makes it harder to have the bigger picture.

Use Class (Data Type) in Conjunction with Variable

You could use class name to provide more context for the variable. One example is for thumbnail. You have thumbnails which have many sizes and maybe a caption to represent it.

You can put loosely in a class,

private final String thumbSmallMainPageUS;
private final String thumbMediumMainPageUS;
private final String thumbLargeMainPageUS;
private final String thumbCapMainPageUS;
private final String thumbSmallSubPageAsia;
private final String thumbMediumSubPageAsia;
private final String thumbLargeSubPageeAsia;
private final String thumbCapSubPageAsia;

which is rather messy.

Or you can put everything in a class that encapsulate the concept and the intention.

public class Thumbnail {
private final String region;
private final String thumbSmall;
private final String thumbMedium;
private final String thumbLarge;
private final String thumbCap;
}
//usage
private final Thumbnail mainThumbnail;

Generalization with interfaces

To improve the cohesiveness of the module, you can proceed to generalize the class. This involves abstracting similar classes by distilling its common behaviors. This in turn (at least in java world for classes) gets you interfaces.

The interface makes it clear that the classes (concrete, implementations) can be used and accessed similarly. In other words, interface makes the intention clear that these classes has explicit contract which states what it has and what it must do (behaviors).

Technically speaking, classes with the same implementation will have to implement the methods in the interface. This allow the class to be substituted with other class that implements the same interface. Looking back at construction vs usage, you can use in conjunction with interface to improve re-usability and clarity of your intention in the implementation.

final ThumbnailBig thumbnailBig = new ThumbnailBig();
final ThumbnailSmall thumbnailSmall = new ThumbnailSmall();

thumbnailBig.resizeMedium();
thumbnailSmall.resizeMedium();

Without interface, the above example ThumbnailBig and ThumbnailSmall will be disconnected by concept. Both of them will be un-related as both are different class. However, conceptually and semantically both are the same. Weeks or months passed, and you look at the code again and wonder, are both of them different to warrant different implementation and contracts?

Utilizing interface, we explicitly make both ThumbnailBig and Small are a type of Thumbnail. Since both implements the same interface (let’s call it GeneralThumbnail), both will have the interface’s method (let’s call it resizeMedium). All types of thumbnail implements this interface and all of them can do resizeMedium. This implementation conveys the idea that the concept of thumbnail is able to be resized to medium which bridges the understanding of the implementation.

Moreover, this is useful in terms of re-usability and test-ability because you can model a concept with multiple implementations and then reuse this in other place as long it has the same contract. That applies for tests as well.

Peer Review

So you put hours, if not days (not weeks :P, we don’t want to annoy your team with a huge PR lol), and now you want to raise a PR and let your team have a look.

Due to mental fatigue or familiarity of your code, you won’t see from perspective of someone with a pair of fresh eye. So things like maintaining method at proper abstraction level or the naming things may be missed in your review. Something that may seem clear to you might be convoluted to others ;).

Someone else may catch this and the team may collective improve the code through feedback and discussion. If you are working alone, then having a break and doing other things then coming back after 2- 5 hours may help you review the code with a fresh mind.

Wrap Up

The above are the basics. Practice it on multiple projects. As it becomes apart of your instinct, you began applying more on what you’ve read and learn.

Making code clean and simple is an evolution process. Step by step, variables, methods and classes being re-defined based on new understanding and models. Tests are the mechanism to ensure refactoring process and changes doesn’t cause regression or unexpected changes in its existing behavior.

Based on above you can see it’s a fundamental block where the code could be evolved to something more structured. Design Patterns for example. The if.. else branches can be improved into chain of responsibility pattern and the construction vs usage into factories.

Key point here is that, intention should be made clear and strive to lower coupling and increase cohesion between all levels of components in a step by step manner.

Reference

1. Clean Code, Robert C Martin
2. Clean Architecture, Robert C Martin
3. Refactoring, Martin Fowler
4. Effective Java, Joshua Bloch