Code base (r)evolution

Over the past years I have worked in many different code bases and have seen a lot of great solutions to problems. Along the way I also have embraced and adapted some of those ideas to make the code better in places where it is not yet in its best form.

In this blog post I want to present a few improvements that can be applied to any code base as you dig in. The changes are small but effective in making the code more readable and more robust in the long term.

🧹 The Tidy First approach

When I start to work on a new feature or a bugfix I usually do a first round of “light” cleanup of the components I’m going to touch. These changes are low risk in a sense that you don’t need stellar code coverage to be safe to execute them. Rather, you rely on the compiler to be your guard rail:

• Check if I can make some methods and properties private (or internal), in order to reduce the publicly visible interface of the component and thus make the boundaries of the component more clear. A simple compile run will immediately tell you whether you are allowed to do the change.
• Check if the import statements are still reflecting the actual usage in that component, removing those that are not needed. This is important for another trick I use, see below. Again, the compiler will help you.

💡 Understanding and documenting

As part of understanding the existing code I write doc strings for all (or most) entities I encounter. This serves two purposes, first it forces me to really think through what a piece of code is doing and secondly, by explicitly writing it out I can figure out if for example a class is doing too many things and should be splitted up. Usually this is the case when a description takes the form of “This class does A and B and in some cases C”.

I use /// or /** … */ for documenting entities that are public (for which a documentation could potentially be autogenerated e.g. using DocC). In-line documentation or internal developer hints that explain reasons for implementation details are documented with // or /* … */ so they will not accidentally end up in an automatically generated documentation.

Along with documenting, I check whether the names of variables, properties, classes and other entities are still correct, if not, I fearlessly rename them until they exactly describe what they are meant to describe. If they are publicly facing, this will be a case-by-case decision. If I get the feeling that it will get out of hands quickly (touching many other components, possibly with different ownership), I’ll postpone the renaming to a separate task.

Unit tests

Then I write unit tests to cover the existing code (if not yet in place), in order to be able to introduce my changes/additions without risk. As a bonus, this will give you a clearer picture of the code’s purpose that you’ll only be able to comprehend by actually executing against the contract, your API so to say.

While working on the unit tests, a pattern that I frequently observe is that tests are unneccessarily long and complex. For example if a mock of a Person object is needed it is recreated completely in all tests, differing only in one aspect, for example the city.

So instead of writing

let personMock = Person(
    name: "Foo",
    surname: "Bar",
    street: "Foobarstr. 25",
    pobox: "1234",
    city: "Foobarhausen"
)

in every test, I’d add a private extension to Person with a static mock() function:

private extension Person {
    static func mock(city: String = "Foobarhausen") {
         Person(
             name: "Foo",
             surname: "Bar",
             street: "Foobarstr. 25",
             pobox: "1234",
             city: city
         )
    }
}

With this the tests will be more readable as they only need to call Person.mock() or Person.mock(city: "Baringen") depending on whether they test an aspect where the city is important.

If I later realise that this mock is needed by more than this test suite it can be consolidated and moved according to the project setup to a folder where project wide mocks reside.

Move away from primitive types

With unit tests as your safety net in place, you can advance to more elaborate improvements.

Some use the term “primitive obsession” for it. I’ve seen it many times - for example with country codes - that are used throughout the code base. Instead, convert the strings to enum cases as early as possible (e.g. when receiving a country code as string from a backend) and use the enum cases in a type-safe way going forward. In the best case you’ll only see the string representation of countries (e.g. CH, DE, AT) in one single place and not anymore thereafter as the respective enum cases will be used e.g. .switzerland, .germany, .austria.

Other indications for primitive obsession that I have observed is that String is used for types which carry more meaning than an array of characters. For example ISBN or VIN. These can be at least type-aliased as a first step to transport more semantic meaning. As a second step they can be extracted as real value objects with validation methods or throwing initialisers that guarantee proper usage of the type.

💉 Dependency injection

Another thing I check is whether dependencies are instantiated within classes and if that is the case (e.g. often seen with NotificationCenter.defaultCenter or WCSession.default or any other Apple Foundation functionality) move them at least to the class initialiser as a simple form of dependency injection. With that change you’ll be able to properly unit test the class as you can inject mock dependencies via a protocol.

Abstraction of 3rd party libraries

Coming back to the cleanup of import statements. In order to get an idea where some module is actually used I like to do a project-wide search such as import GoogleMaps which will give you an idea if the 3rd party module is actually abstracted or if its usage is scattered accross the code base. This requires that the import statements are in a “clean” state, otherwise you’ll get a lot of false positives. In the best case you’ll only get one or a handful of hits for that search. If you get more hits you should introduce a layer of abstraction between your code and the 3rd party library. This abstraction can be “dumb” in the beginning, meaning that you can almost copy the interface and pass on calls to the library. However I’d suggest to already try to be more generic and not leak any library code into the interface and your code by providing your own models for data exchange with according data mappers (that can be conveniently added as extensions of the models).

Small methods

Reading through a lot of code it is much simpler when the methods are short. Long methods with several code branches are inherently difficult to comprehend. When I encounter such long methods I try to split them up by extracting inner parts into separate (private) methods, safeguarded by enough unit test coverage of course.

A more semantic boolean negation

Finally, a small gem 💎 that can increase readability is this: The boolean negation ! character is so small it can be overlooked easily in the context of some bigger expression it’s applied to. A mini extension for the Bool type can make it more obvious for those occasions where a negated boolean can’t be avoided:

extension Bool {
    var isFalse: Bool {
        !self
    }
}

let mybool = true
mybool.isFalse // instead of !mybool

🏁 Conclusion

I have presented a few simple cleanup techniques that can be performed even without unit test coverage by simply letting the compiler work for you. These changes do not take much time and can be applied alongside the day-to-day work that has to be done. The more advanced improvements need decent coverage to not introduce regressions, but can significantly improve the readability of the code.

• Tags:
• Refactoring
• Legacy-Code
• Software-Evolution