Over complexity

I heard such an expression that in order to become a programmer, you need to be lazy. But sometimes laziness in programming leads to a terrible technical debt. In my article on SRP, I mentioned that violating this principle can lead to an increase in complexity or even to multiplication of it. One of my colleagues produced an interesting example, and with my help I decided to demonstrate how it looks.



Let's decide what is this excessive complexity. But first, let's talk about its opposite, about the complexity of the requirements. For example, there are requirements to calculate the employee’s salary from the hourly rate and hours worked. And, if an employee has been working in the company for more than five years, accrue a bonus. This “if” comes from the requirements, and it cannot be avoided. In one form or another, it will become an element of complexity in the application code, most likely in the form of a conditional operator “if”. But sometimes the complexity does not proceed from the requirements, but follows from the developer's approach to solving the problem.

The “if” operator, patterns such as “strategy”, polymorphic methods are not a complete list of programming techniques that can contain this excessive complexity. Personally, I, by the way, am always against the use of patterns by developers simply because they can, and not to solve a specific problem.

Here is a simple example. It may seem fictitious, but it is not. It is not even simplified, it was in this form that I met him during a code review a couple of years ago. In two places in the code there were calls to the same function but with a different Boolean parameter:

// first place
doSomething(true);
// second place
doSomething(false);

Similar designs always look suspicious and this function did not disappoint me. This parameter was passed for the sole purpose to be checked inside this function:

doSomething(flag: boolean): void {
  if(flag) {
    // do first thing
  } else {
    // do second thing
  }
}

This check can be described as "if I was called from place A, we do one thing, otherwise I was called from place B, we do another thing." This flag, this “if” is what this whole note is about. The complexity does not come from business requirements. Naturally, I recommended changing the code as follows:

// first place
doFirstThing();
// second place
doSecondThing();
//method is split into 2 parts each having their own responsibility
doFirstThing(): void {
  // do first thing
}
doSecondThing(): void {
  // do second thing
}

That's all, there’s no more complexity. This is where the developer should not be too lazy and write another function signature.

Here you can exclaim: “But this is just one 'if'”, or: “This violation is obvious, who writes the code like that?” And here comes the second example. It shows that it can be noticeably more difficult to see the violation, and also that the cost of this violation can be more than just one “if”. As in the first example, the function is used in two places:

// first place
checkValidity(obj);
// second place
checkValidity(arrayOfObjs);

The method, as follows from its name, checks the validity of the object. However, it was not obvious that it could also check the validity of an array of objects. I tweaked the variable names to emphasize this violation. The method looks like this:

checkValidity(parm: MyType | MyType[]): void {
  if(Array.isArray(parm)) {
    parm.forEach(p => checkValidity(p));
  } else {
    // here the object gets checked 
    // and conditional exception is thrown
  }
}

Here it is. One if becomes a lot of ifs. If the array contains 100 objects, then this “if” will execute 101 times. And on real data, we could have 30 thousand objects there, and this is already an impressive performance loss.

Obviously, following the principle of sole responsibility, this method needs to be refactored so that 2 methods are obtained:

checkItemsValidity(parms: MyType[]): void {  
  parms.forEach(p => checkItemValidity(p));
}
checkItemValidity(parm: MyType): void {
    // here the object gets checked 
    // and conditional exception is thrown  
}

Correspondingly, you also need to adjust the call points.

It is interesting that the examples that I gave in the article about SRP led to an increase in SLOC, the same examples, on the contrary, lead to a slight decrease in it, along with the expected improvement in the quality of the code.

That's all. Just a couple of simple examples to demonstrate the most important principle of good code.