What is code smell?

Code smell means that if a code looks awful then it most likely it is buggy and hard to maintain. So a code smell code is a code of low quality. There are several rules about what is a good code and what is a bad code.

For example, the next two rules:

• fields must be encapsulated

• methods and constructors must have at most 3/4 arguments.

There are other rules as for the size of a method and even to use 80 columns even when currently it is really hard to find a screen that its not wide and even a 10 year old computer could show more than 120 columns.

For example, it is a code in Java:

class Customer {
   public int id;
   public String name;
   public String surName;
   public Address address; /* address is some class */
}

If you look at this code, you will notice that it is broken an iron clad of JAVA: Fields must be private and they could be encapsulated!. That is a smelly code.

While:

class Customer {
   private int id;
   private String name;
   private String surName;
   private Address address; /* address is some class */
   public void setId(int id){
      this.id=id;
   }
   public int getId(){
      return id;
   }
   public void setName(String name){
      this.name=name;
   }
   public String getName(){
      return name;
   }
   public void setSurName(String surName){
      this.surName=surName;
   }
   public String getSurName(){
      return surName;
   }
   public String getAddress (){
      return address;
   }
   public void setAddress(Address address){
      this.address=address;
   }
}

That is the correct code. Right?

The answer is: it depends, and that is when the ideology of Code Smells falls apart.

There is always a reason to do or avoid doing something. There is no a silver bullet that solve every problem (one solution fits all myth).

In the case of the encapsulated code, the IDE could generate the encapsulation (because it is tedious to write it over and over again). But that is part of the problem.

For example, let's say, our class has a field Address, this field has the address of our customer and inside it, there is a field with the city and we want to change the city.

// using the class with public fields:
Customer cus=new Customer();
cus.address.city.idCity=20;
// using the encapsulated class
Customer cus=new Customer();
cus.getAddress().getCity().setIdCity(20);

The first code is easy to understand, while the second is long and hard to follow.

Lombox to the rescue, or not?

We could write the same class using a library called Lombox.

@Setter
@Getter
class Customer {
   private int id;
   private String name;
   private String surName;
   private Address address; /* address is some class */
}

It resolves our problem to write the setter and getters.

However, it brings a new problem. Lombox works as a pre-compiler:

• You compile your code.

• Before it is compiled, Lombox takes your code and converts it into Java Code

• And finally, you compile the Java code generated by Lombox.

Great? Well, not really. What if you have another pre-compiler code and your pre-compiler code calls a setter or getter:

Customer cus=new Customer();
cus.getId(); // this code is only valid after Lombox compiled the code.

So, Lombox solves 1 problem in exchange for another problem. Since Lombox is "magic code" (it works under the hood), then when it works, it works fine but when it fails, it is really funny to debug.

Another solution.

Let's say we decide not to use setters and getters and use instead a constructor.

class Customer {
   private int id;
   private String name;
   private String surName;
   private Address address; /* address is some class */
   public Customer(int id,String name,Sting surName,Address address) {
         // more code
   }
}
Customer cus=new Customer(1,"john","doe",add);

But it is against the rules: (to have a constructor with more than 3 arguments)

So we can't win without creating a lot of code that usually will never use it.

So, why the rule?

The rules consider that every code does the same it is not even true. And the worse, it is making programming boring and not funny.

Of course, some people are overly creative with its code but it should be a middle ground, i.e. to use common sense.

What happens if our code is complex?

We have two alternatives, SRP (i.e. split the problem into mini subproblems) and/or comment on the code.

Commenting on the code does not hurt.

The myth also says that the code must be self-explaining. Well, not really. For example the function find-a-text-inside other. It could return the position (if the text is found). But what if the value is not found? Maybe it could return -1, null, or an exception? It depends on the code and it is a PITA to check what this code returns.

It also happens with a timeout function

timeout(int interval)

Is the interval a second? a millisecond? or what? A lot of languages use milliseconds, so timeout(2000) means 2 seconds but other uses seconds, so timeout(2000) means half an hour.

And about the example, is it not easy to call the argument "second" or "millisecond" instead of "interval"? Yes, but most methods are written in that way.