Photo by Debby Hudson on Unsplash

We all write bad code. Let’s face it. Even if we do our best at writing the most efficient and well-designed code, we all happen to write this dirty method we don’t feel so proud of and dread having to debug next month, or next year.

I’m not going to talk about that code, because it’s so easy to spot and there is no need to write blog posts about it. What I’m interested in is code which looks fine when you have finished typing, but will bite you in the back someday. It ends up being hard to maintain, extend or even understand. After years of coding, I have come up with a set of warnings that alert me whenever I am heading for a dead end.

Meaning

Code is language. Even more so when you use a programming language such as Ruby, which is designed to be very expressive. But to me, code begins to smell when it becomes unclear what it really does.

Names

Naming is essential. One of the thing that changes the most in my creation process is probably the name of the classes, methods and variables I design.

Humans name things they understand. And the higher the knowledge of an element, the sharper the naming. Renaming my classes in the development process is usually a good sign that I am getting a better idea of what they do or what concept they stand for.

Method length

One of the most frequent design patterns we are taught as aspiring OOP developers is to keep methods shorter than a few lines. One caveat, however, is that code ends up scattered all over the place, and you can sometimes lose touch with the lifecycle of your data. This is especially true for classes that are used to build a data structure.

For instance, take this class that builds a data hash to be sent to FCM:

module MobilePush
  class FcmNotification < BaseNotification

    def launch
      # Bootstrap stuff...
      push_service.send_message(project, data)
    end
    private
      # Lots of methods...
      def data
       {
          notification: json_notification,
          recipient: recipient_name,
          room: room_name
        }
      end
      # Lots of other methods...
      def json_notification
        @notification.to_json
      end
      def recipient_name
        @recipient.name
      end

      # Maybe a few other methods...
      def room_name
        @room.name
      end
      # The rest of the methods...
  end
end

Note: This example is a little over-simplified for brevity. But you get the idea.

I wrote this thinking “Hey that’s smart! Everything is nicely sorted. It really looks like clean code”.

Then I took a few day’s break with my family. While I was away, one of my co-workers had to deal with my new code and add a new feature. But he found it unnecessarily abstracted, hard to maintain and extend. And he was right. So I refactored it to:

module MobilePush
  class FcmNotification < BaseNotification

    def launch
      # Bootstrap stuff...
      push_service.send_message(project, data)
    end
    private
      # Lots of methods...
      def data
       {
          notification: @notification.to_json,
          recipient: @recipient.name,
          room: @room.name
        }
      end
      # The rest of the methods...
  end
end

So OK, I had to swap out room_name for @room.name in two or three places in the class, because it was also used elsewhere. But I also made my code a little less abstracted. Now, my co-worker knows exactly what data is made of, at a glance.

Stories

One thing to remember when writing code, is that it is not just meant to be effective and concise. Code is meant to be read by others. Your co-workers, your team leader, and even more crucially, by the person who will be in charge of your once-new, now-legacy codebase.

One-liners

I love one-liners. They are fun, straight to the point, and they showcase the technical skills and the wit of their creators. They are to programming what punchlines, or newspaper headlines are to writing. But who wants to read a whole book written in this way?

Even though I sometimes use one-liners in my code, I only do so when I feel confident my successor - or my future self - will be able to make sense out of it easily.

Be explicit

I believe that most code should be explicit and guide the reader with landmarks that will help him or her follow along.

For instance, instead of this kind of expression:

Device.where('last_seen > ?', 1.hour.ago).where(logging: true).where(notifications: true).last.send_message(topic, payload)

I would rather either use class methods, or scopes in the model, to end up with:

device = Device.active_last(1.hour).logging.notifying.last
device.send_message(topic, payload)

I’m not stupid. I can understand the first example. But when parsing hundreds of lines of code to find a bug, I can look at this code and instantly tell what it does. No need to make sure I got the < and the > right. This was done once, on the day it was typed, and I can rely on it with confidence because each of these methods is tested and does what it says.

Be methodic

I like to separate intents in what I do with my class. When you tell a story, you first introduce your character with a clear description. Then only you give him an active role in your story. That’s what the code example above is doing. I could also have easily written:

Device.active_last(1.hour).logging.notifying.last.send_message(topic, payload)

But that’s mixing intents. This line is both querying the model and triggering an action, so you start reading the line thinking: “Oh, he is defining the Device” and finish it thinking “Ah. No, he is sending this device a message”. Your reader should never ever be misguided, even in short, simple assertions like these.

I try to separate queries from actions. They are two types of tasks and require two distinct steps in my code.

Consider these three sentences:

The man in the bar that is old and dirty is talking about yesterday’s soccer results. - messy

There is a man who is talking in the dirty old bar about yesterday’s soccer results. - clearer

In the dirty old bar, there is a man. He is talking about yesterday’s soccer results. - crystal clear

I want my code to be crystal-clear. I want my code reviewer, my successor or my future very-tired-on-a-friday-afternoon self to perfectly get what I am doing with my code.

Tests

Writing tests helps me identify dependencies. Integration tests often require quite a lot of bootstrapping: a logged in user, a few related entities… that’s fine.

But I sometimes write tests that rely too much on other models. If you can’t test your Comment model without a Post model, then something may be too tightly coupled in the code, and the relationship between those entities needs to be inquired.

Assertions also guide me towards code clarity. I have found that when I have trouble defining an expectation in my test, it usually means that this particular method is not doing something clear enough. It either needs to be split into smaller methods or completely re-thought about.

A word about comments

Another sign that code needs to be refactored is comments. Comments mostly indicate that what the program does is unclear. So when I see comments in code, the first thing I do is delete them and figure out what is happening without reading them. And whenever that comes out messy, it’s a good sign that they can all be replaced by a single TODO: refactor.

Comments are also often a sign of an external dependency. Typically, you will get this kind of thing:

# Handle temperature > 21
# Deal with legacy customer IDs

The first one might depend on the device you’re dealing with, and that dependency might be addressed by the hardware team.

The second one could be deleted by resorting to an inherited class, like LegacyCustomer, which could handle that kind of special user cases and keep the Customer class clean with a predictable and simple interface.

The only comments I keep are top comments above class declarations, Yard documentation, a few refactor TODOs and silly, fun meta about the code.

Conclusion

I sometimes look at messy code and wonder “who wrote this piece of crap?”

Let’s see… Git blame…

Me.

I don’t like this feeling. I want to feel proud of my code. I want other developers, including my future self, to make sense out of it easily and instantly, and thank me for making the effort of crafting code that makes sense, is easy to debug and maintain.

Following this set of guidelines will hopefully help me down that path.