Identifying the dirt in our code - names, functions, and comments

rachc

Rachel Curioso :D

Posted on September 29, 2019

Identifying the dirt in our code -  names, functions, and comments

The problem of having code that is not very clean is that, as we maintain it, the code will get more and more complex and its understanding become compromised.

When I talk about complex code, I don't mean business rules, but the complexity of understanding and code legibility. I'm talking about spending some time trying to understand what is p or what the read(p, i)function do when you don't have any context. What does it read? what are p and i? Aaaaahhh /o\

When we are in a place that we spend more time trying to understand an old code than thinking on the new code implementation, it could be a sign that something could be better. Spending more time in a task than we liked to, leave us frustrated

When is difficult to maintain code, we stop to care about its quality and consequently we write "bad" code. We leave the code even more complex and, the more complex it gets, and more difficult it is to maintain. Our productivity drops almost to 0 and it becomes risky to fix a problem without developing 3 more on this process. We become afraid to touch a code that already works "magically" (because we, obviously don't have a clue about what's happening)

The consequences of this are code so difficult to maintain that companies end up closing. I've seen it happen once. The only product had several bugs, it was impossible to fix a bug without creating a few more and the customer satisfaction has reached a level that they didn't want use the product anymore, and the company closed

If we know that is so harmful, why we do this?

Hurry.

Usually, we have such tight deadlines that we deliver our tasks poorly to skip to the next.

I want to avoid this from happening. How I do this?

This is the first part of a series about clean code. The idea, in this first topic, is talk about naming things, write better functions and a little about comments.

Next, I intend to talk about formatting, objects and data structures, how to handle errors, about boundaries (how to deal with another's one code), unit testing and how to organize your class better.

I know that it'll be missing an important topic about code smells, but this is a huge topic that has the potential for another series.

So, let's begin?

Give good names!

gif escrito "hello, my name is"

Use names that show what your code does:

int d could be int days

instead while x == 7, we could use while time == DAYS_OF_WEEK.

Avoid misinformation.

if accountsList is not a list, it should have a better name.

Varible that has very subtle name differences also fall in this topic, such as
VeryNiceControllerToDealWithStrings and VeryNiceControllerToStoreStrings.

How long did you take to spot the difference between the two variables?

Make a good distinction of your variables.

If we have two variables, one called banana e another called theBanana, how can we differentiate which one is which? This same rule applies for CustomerData, Costumer and CostumerInfo. What's the difference between then?

Use names that you can pronounce

It facilitates internal team communication. If you pair, you don't need to refer to a variable as wpdw. A workDaysPerWeek sounds better (:

Use searchable names

Have you ever tried to use ctrl+f on a variable that you couldn't remember the name? Or that it was so common that you just couldn't? Have you tried to search for a variable called f?

Avoid prefixes!

It's another thing that makes the search difficult. Not only the search but mainly autocomplete. if the variables start with icsd, like icsd_name or icsd_date we'll have a hard time to find what we are looking for.

Avoid mind mapping

When we use a huge quantity of variables that don't make much sense, we need to memorize what they are meanwhile we are trying to figure out what the code does.

user.each do |u|
  u.each do |s|
    s.each do |h|
      print h.name
    end
  end
end

(this code block has another problem with so much each inside each inside each insi.... but we'll talk about that later)

Class names

Should be nouns, and not verbs, because classes represent concrete objects. They are the representation of things.

Methods names

Should be verbs, and not nouns, because methods represent actions that objects must do.

Add context!

If you bump on a variable state alone, probably will have a hard time to figure out what it means.

But if this variable is near street and number it's easier to understand what it does.

Add context, but not for free.

Let's suppose that we are working on a project called chocolate ice cream. We don't need to add CIC in front of all variables.

At the same time, inside a User class, we don't need to call all the variables userAdress, userAge or userFavoriteColor.

Functions

The swedish chef drumming on the pan

They should be small

They should be small and readable, with few nests. In the ideal world, they should be 1 or 2 levels of indentation.

Do only one thing.

They should make only one thing and that's it.

But how do I know that it does only one thing?

def verifyNameAndLogout
  if rightName?
    showMessage
  end

  logout
end

If you look at the code, it seems that it does 3 things

  1. Verify if the name is right
  2. If yes, it displays a message
  3. logout, regardless the name is right or wrong.

We understand that it does what is proposed in only one level inside the function, so we can say that this function do only one thing.

The only modification that we could do in this code would be relocating the if statement to another function, but in this case, it would be only a relocation, without any additional advantage

Functions can't be divided into sections. If you can divide, it's a sign that it could be more than one function

One abstraction level per function

When we talk about abstraction levels, we can classify the code in 3 levels:

  1. high: getAdress
  2. medium: inactiveUsers = Users.findInactives
  3. low: .split(" ")

The ideal is not to mix the abstraction levels in only one function. When we mix, the function becomes a little harder to understand, and we can't read the code step by step, from top to bottom, like an article

The best scenario is that you can read code from top to bottom, like a narrative.

ex:

  1. To include the setup and the teardown, we first include the setups, then we include the page content, and after that, the teardown.
  2. To include the setup, we add the setup suite and then the default setup.
  3. To include the test setup, we search for the hierarchy...

(and so on...)

Avoid switch statements and excessive if/else

The motive that we should avoid extensive switch/if-else is that beside them beeing to big and doing many things, they violate the open-closed principle of object-oriented programing or. also known as the O in S.O.L.I.D .

One entity should be closed for modification and open for expansion. When I talk about entity, I mean class, module, function and so on

When there are several if/else/switch in a single module, we have to modify the code if we want to add any extra case, what makes the code more fragile and difficult to test.

Sometimes we need to verify differents situations and take different actions depending on the situation.

One way to decrease the switch/if/else is to use polymorphism.

Using a simple example:

foreach (var animal in zoo) {
  switch (typeof(animal)) {
    case "dog":
      echo animal.bark();
      break;

    case "cat":
      echo animal.meow();
      break;
  }
}

this could be refactored in this way:

foreach (var animal in zoo) {
  echo animal.speak();
}

On this example, under the hood, we created an animal class, that has the method speak. Then we created a dog and a cat object, we give then the behavior to speak and that's it.

Sometime, polymorphism could be a little too much to solve this problem. Sometimes we rearrange the logic, create a few more function and fix our problem.

Less arguments is the way to go

When we talk about arguments, Less is more

When we have many arguments, the complexity of our function and, consequently, our tests greatly increases.

Another thing that happens is mistaking the argument order when we call the function.

Occasionally we can decrease the argument size creating a class.

A function with several arguments could be transformed in a class, and this function could be a method in this class. The arguments, much of the time, could be their properties and, this way, we don't need to pass any argument.

In some cases, we don't need this much, so we could use other ways to decrease the number of arguments, like grouping then.

Circle makeCircle(double x, double y, double radius) could be transformed in Circle makeCircle(Point center, double radius)

Function are verbs, arguments nouns

When we name function as verbs, and arguments as nouns, it becomes more legible and easier to understand the behavior that we expect, like write(name)

The argument order is also important so you don't forget when you'll use the function.

Just imagine to get the order right if the arguments were changed: assertExpectedEqualsActual(expected, actual)

Avoid flag as arguments.

Avoid passing a boolean as an argument, because it makes the refactor hard.

Try to verify the truth or falseness when you call the function, and not inside it.

A function shouldn't have collateral effects.

If we have a function called checkPassword, it shouldn't do a login inside its body.

We were not prepared to login at any time and this could cause undesirable side effects, besides test de function becomes more difficult.

Output arguments

Output functions should be avoided. If a function needs to change the state of something, it should change the object.

Sometimes we bump into functions like appendFooter(s) and we are not sure if the argument is an input (s is the footer and we are attaching it somewhere?) or output (we will attach s to the footer?).

If the first case is the correct one, if s is the footer, the best thing to do is objectInstance.appendFooter

Command Query Separation

A function should change the state of an object, or return some info about it.

This is an example of a function that doesn't follow the command Query Separation:

if name?
  name.change
  true
else
  false
end

A function that changes something and returns true or false should be replaced by two. One that verifies, and another that makes the change.

DRY - Don't repeat yourself

The problem with repeated code is, besides that it inflates the code, any change that we need to make we have to change in several places.

However, we must be aware that too much DRY can be harmful. We often confuse code duplication with business rule duplication. Sometimes it's okay to duplicate a code as long as you're not duplicating a business rule that should be unique.

There is another maxim also that says: you must write the same code a maximum of 3 times. The third time you should consider refactoring and reducing duplication.

Avoid returning an error code

When methods return error messages, it violates subtly the Command Query Separation. When we put an error inside an if block, it may cause some confusion, since it may be returning something or an error.

if name?
  name.change
else
  raise 'An error has occured'
end

In the example above, if it works, it performs an action. If it went wrong, it pops a mistake

Comments

Dog beeing interviwed. Subtitle says "no coments"
Avoid

Comments on a code usually bring more evil than good. Most of the times, a comment about what a variable does, or details of how the things work could:

  1. Become outdated, confusing your future self (Yesterday I lost some precious time because of an outdated comment)
  2. Could be replaced for some better named variable/function/class.
  3. They pollute the code unnecessarily.

It should be clear that avoid != forbid. If you are a java programmer and is doing something public, Javadocs is important. Sometimes it's interesting to explain some code (How many time you spend trying to figure out a regex pattern when you see one?), but, in 99% of the time, comments could be avoided.

How I will write the code if I have to pay attention to all these rules?

Surprised Pikachu

In a good article you put the ideas on a paper and goes refining the text. coding functions in the same way.

First, you make it work, then you refactor.

Uncle Bob, in clean code, defends that the best order to write code is:

  1. Write unit tests.
  2. Create code that works.
  3. Refactor to clean the code.

Write all the tests, make it work and, when you are sure that everything is the way that's supposed to be, refactor removing all the dirty code and applying design patterns.

Particularly, I don't have the habit to write tests before the code (TDD), however, test the code before doing a refactoring is indispensable. Only then we can be sure that your code will always work, even after so much modification.

The Clean code book is focused on bad practices. It details several problems and does not delve into ways to solve them.

If you want to delve more into how to fix the dirty-code-problem, we could look into the book refactoring. In this book, there are details and several different ways to eliminate the dirty in the code

💖 💪 🙅 🚩
rachc
Rachel Curioso :D

Posted on September 29, 2019

Join Our Newsletter. No Spam, Only the good stuff.

Sign up to receive the latest update from our blog.

Related