Short conditionals - Clean Code principles

It’s hard to write about programming without mentioning this “holy bible” of developers - Clean Code: A Handbook of Agile Software Craftsmanship by Robert C. Martin.

I don’t remember when I read it for the first time. Long after that I bought paperback version and last month reread few parts of it. I’d like to show few things that I did in code (and unfortunately still do, sometimes) that are described in Clean Code as bad habits. I will try to write some short blog-posts based on paragraphs from Uncle’s book, and give some examples.

In the examples I intentionally don’t use ternary operators - one liners are pretty, but sometimes harder to read for beginners than plain old if-else conditional.

So, lets go!

Introduction - if-else hell

One of the beginner programmer sin is creating if-else hell, like this:

if(item.price > 0) {
  if(item.mass > 0) {
    if(item.tax > 0) {
      basket.add(item);
    } else {
      throw new NoTaxException();
    }
  } else {
    throw new NoMassException();
  }
} else {
 throw new NoPriceException();
}

It’s hard to read such code, not saying a word about understanding it. It’s way better to check each condition with separate block like this:

if(item.price <= 0) {
  throw new NoPriceException();
}
if(item.mass <= 0) {
  throw new NoMassException();
}
if(item.tax <= 0) {
  throw new NoTaxException();
}
basket.add(item); 
}

It would be even better to extract conditional blocks to validation method inside Basket class and use try catch block, something like this:

try {
  basket.add(item);
} catch (error) {
// error handling
}

add(item: Item) {
  if(this.price <= 0) {
    throw new NoPriceException();
  }
  if(this.mass <= 0) {
    throw new NoMassException();
  }
  if(this.tax <= 0) {
    throw new NoTaxException();
  }
  this.list.push(item);
}

Conditional operator’s blocks should have only one line

When you split if-else hell, you can go into few more traps.

One of such trap is considering main logic inside an “if” block, and pushing special case to the end. For example:

const data = findData();
if(data) {
  // Main logic
  const transformedData = transform(data);
  console.log("Data transformed");
  return transformedData;
}
// Special case
return [];

When you come to last line, you will often ask yourself - why is empty array here? Why is it returned? Then you will have to skip back and find “if” that begun this conditional - sometimes many lines earlier.

It’s better to do this other way around - start with special case handling. After checking the special case you can clear mind from conditional and resolve the main logic:

const data = findData();
if(!data) {
  // Special case
  return [];
}
// Main logic
const transformedData = transform(data);
console.log("Data transformed");
return transformedData;

Next part is about that special cases.

Conditional cases should be extracted to separate methods

Sometimes that special case contain some additional logic, maybe even more complicated than the main one (errors handling, failure notifications, cleanup). If it is the case, you will try to fit that logic inside “if” block like here:

function provideData(): OutputData {
  const data = db.findData();
  if(!data) {
    // Long process of data generation
    const dataGenerator = new DataGenerator();
    const generateData = dataGenerator.generate(); 
    ...
    ...
    ...
    return generatedData;
  }
  // Also long process of data transformation
  const transformedData = transform(data);
  console.log("Data transformed");
  return transformedData;
}

Of course, in first iteration it’s not very bad to “save” your thinking like this, but you should refactor this quickly, before it grows into a monster. Move blocks to separate methods, and code will become longer, but more clear.

function provideData() {
  const data = db.findData();
  if(!data) {
    return generateData();
  }
  return transformData(data);
}

function generateData(): OutputData {
  // Long process of data generation
  const dataGenerator = new DataGenerator();
  const generateData = dataGenerator.generate(); 
  ...
  ...
  ...
  return generatedData;
}

function transformData(data: InputData): OutputData {
  // Also long process of data transformation
  const transformedData = transform(data);
  console.log("Data transformed");
  return transformedData;
}

Last thing, I find related - try to avoid “if-methods”. It might be tempting to hide conditionals like this:

data.generateData(input);
data.transformData(transformer);

// Data.ts
export class Data {
  generateData(inputData: InputData) {
    if(inputData.length === 0){
      //do generate
    }
  }

  transformData(transformer: Transformer) {
    if (this.isTransformable()) {
      // do transform
    }
  }
}

When looking at the main source you will think, that generation and transformation is always performed. And then - what a suprise, there is conditional inside method. You have to check each one, to know if it is doing what you think it is doing. Don’t make your companions jump around code only to find out that method is not resolved because of simple conditional. Explicitly resolve conditional before launching method:

if(inputData.length === 0){
  data.generateData(input);
}
if (data.isTransformable()) {
  data.transformData(transformer);
}

// Data.ts
export class Data {
  generateData(inputData: InputData) {
      //do generate
    }
  }

  transformData(transformer: Transformer) {
      // do transform
    }
  }
}

Summary

Those are few basic thoughts from Clean Code - Functions chapter, and only few for the beginning. When you read “the Bible” paragraph by paragraph you can gather much insight into your coding style. Of course, those are principles that you can agree, or disagree* with - but you always should think about why you do so. I find some of them really useful, when some of them are just old-style.

*Sample of such disagreement: https://copyconstruct.medium.com/small-functions-considered-harmful-91035d316c29