Don’t be anti-negative

The negative conditionals in code blocks are most of the time difficult to read and understand.

It becomes worse when you make the conditionals anti-negative.

Before

Anti-negative conditionals

Try to read above code snippets and try to understand it. Please note the highlighted parts. Does that code reads like

  • When inverse of a package is not damaged then report an error: “Package is damaged”.
  • When inverse of a package is not empty then report another error:
    “Package is empty”

Ouch! Unreadable? Complex? Painful, isn’t it? Let’s revisit the same condition with positive conditionals.

After

Positive conditionals

Another example:

Before:

Set hours logged when it is not NonWorkingDay. What?

Above snippet is showing hours worked for the day if it’s not non-working day!

After

Set hours logged when it was working day.

Now, this reads like simple statement. Show hours worked for working day.


Extract Method

Extract method is the most frequently used refactoring technique. It reorganizes the code for better reuse and readability.

Ah! alright. Let’s proceed.

Use the extract method in following cases —

When your class contains a long method.

A long method does more than one thing and it is not considered as cohesive. The method more than 10 lines could be considered the as long method.

See this example of a long method, It is doing more than one thing.

Setting up GST rates, item/category mapping, accepting user inputs, calculation and so on. 

A long method is considered a code smell. 

Now see the refactored version of the above long method.

It is divided into small, cohesive, fine-grained methods which are doing one and only one thing.


When you favor a comment to express the intent of the code.

Before

 ...
// Parse input and convert it into Document
var xDocument = XDocument.Parse(input);
var document = new Document
{
   Title = xDocument.Root.Element("title").Value,
   Text = xDocument.Root.Element("text").Value
};
...

Here let’s get rid of comment [Parse input and convert it into Document]. 

After

...
var doc = ParseInput(input);
var serializedDoc = JsonConvert.SerializeObject(doc);
...
private Document ParseInput(string input)
{
    var xDocument = XDocument.Parse(input);
    var document = new Document
    {
        Title = xDocument.Root.Element("title").Value,
        Text = xDocument.Root.Element("text").Value
    };
    return document;
}

When the extract method helps you to improve readability.

Do you know that people extract method for a single line too? Because it adds to the readability of the code. Sometimes a statement may be difficult to interpret. In that case, you could use the extract method to convey intent instead of taking the aid of comment.


When there is a code duplication or copy-paste.

Consider below example, it is formatting date and code is duplicated. Perhaps, lines are copied and pasted.

Before

    var formattedStartDate = startDate.ToString("dd MMMM yyyy");
    var formattedEndDate   = endDate.ToString("dd MMMM yyyy");

As soon as you see copy and paste of lines of code then go for an extract method refactoring even if it is a small portion of code.

After

    var formattedStartDate = FormatDate(startDate);
    var formattedEndDate  = FormatDate(endDate);

public string FormatDate(DateTime date)
{
   return date.ToString("dd MMMM yyyy");
}

The advantage

  • Code becomes more readable.
  • Fine-grained methods make code easier to understand.
  • You can easily plug-in and plug out the implementation for some routines.
  • When your method becomes cohesive, you can easily spot the bugs in the code.
  • Your calling method reads more like a sentence.

Points to Ponder —

  • Remember, keep the method short, well-named, and cohesive to get more benefits out of it.
  • Your calling method should read more like sentences.
  • When extracting a method, a developer should think about the cohesiveness of the class. should ask a question to oneself: Does the extracted method really belong to this class? If Not, then move it to the appropriate class that should perform that operation.
  • Small methods really work only when you have good names.
  • Don’t just extract method if you can’t give the meaningful name.

Extract method only if you can give an intuitive name.

Don’t be too clever

A few days ago, I along with my friend went to the restaurant. When I went to use a washroom, I found these symbols on the doors.

I realized the humor of the designer and then I went to the Male washroom. However, I observed that other people who don’t understand the humor, having difficulty to understand the correct washroom. They had to ask staff for help. Hmm! That’s not that user-friendly.

Similarly, people try to be too clever while writing the code.

Please see the above code snippet. Do you understand the intent of BombDetector.Detect? Original author must have tried to use his humor while writing the code.

However, the maintenance developer may not understand the humor behind it and he would feel helpless to understand the intent.

The original author was checking the value of the input argument and throwing the exception if a value of an argument is null.

Above snippet could have been kept simple.


Try to keep your code as simple as possible so that everyone could easily understand it and easily work with it.

Don’t add unnecessary context

This is about a class, which has fields and methods whose name repeats the class name.

Please see the below class diagram for a student.

Looking at the class diagram, it’s clear that this whole class is all about the student. However, do you think repetitively adding the word “Student” in fields and methods really adds value?

Don’t you think it’s redundant?

It reads more like

  • The student has a student name.
  • The student has student birthdate.
  • Get student age of a student. It reads like a weird sentence, isn’t it?

We already have the student context, hence adding words repeatedly is redundant.

Let me show you the revised class diagram.

This one removes the redundancy. 

It reads more like

  • The student has a name.
  • The student has a birthdate.
  • Get the age of a student.

You may come across such redundancy when working with code. Identify it and remove it.

Next time, “Don’t just add unnecessary context”!

Another example:

Student repository

Don’t repeat the name of the class in class’ fields and methods.

Naming boolean variables

The boolean variables in codebase should read more like a question that answers either yes or no.

If it doesn’t answer you in ‘yes’ or ‘no’ format, then please revisit naming.

Below are some of the bad examples which feel like commands and I don’t think they are really answering the question in either yes or no format.

Dirty

  • emptyUrl
  • openFile
  • startJob
  • closeConnection
  • logIn

They feel more like verbs/actions/commands. For example emptyUrl. In my opinion, it conveys that it empties an URL. Similarly, openFile seems more like a command that opens a file.

Let’s see some of a good example of naming the boolean variables.

Good

  • isUrlEmpty
  • isFileOpened/hasFileOpened
  • isJobStarted/hasJobStarted
  • isConnectionClosed/connectionClosed
  • loggedIn

Are these variables answering in yes/no format?

While naming boolean variables, you could use verbs like is, has, can, was, contains and so on that could be part of a variable name.


More Examples:

  • isAdminUser
  • hasInvalidCharacters
  • canUploadFile
  • wasPackageDamaged
  • containsBadWords
  • containsDamagedItems
  • shouldCreateSession
  • isComponentMount…

Don’t use abbreviations while naming variables

A few months ago, Technogise had conducted a Code Retreat session for engineering graduates. I was one of the volunteers for this activity.

We asked students to solve a simple problem.

We asked them to write a program which calculates the final price of product items in a shopping cart, when the initial price of each item, the category in which each item falls, their quantity and the GST (goods & services tax) slab applicable to each category was given.

Going through the code for some of the students led me to write this post.

Please read the code below:

using System;
using System.Collections.Generic;
using System.Linq;
namespace Assignment
{
internal class Program
{
private static Dictionary<string, int> CGR = new Dictionary<string, int>();
private static Dictionary<string, string> iCat = new Dictionary<string, string>();
private static void Main(string[] args)
{
CGR.Add("Food-grains", 0);
CGR.Add("Furniture", 5);
CGR.Add("Electronics", 18);
CGR.Add("Cosmetics", 28);
iCat.Add("Rice", "Food-grains");
iCat.Add("Wheat", "Food-grains");
iCat.Add("Sofa", "Furniture");
iCat.Add("Chairs", "Furniture");
iCat.Add("TV", "Electronics");
iCat.Add("Mobile", "Electronics");
iCat.Add("Shampoo", "Cosmetics");
iCat.Add("Perfume", "Cosmetics");
Console.WriteLine("Welcome to NMart store");
Console.WriteLine("***************************");
Console.Write("Enter name of item: ");
string iName = Console.ReadLine();
Console.Write("Enter quantity of item: ");
int iQnt = int.Parse(Console.ReadLine());
Console.Write("Enter rate per product item: ");
int iRate = int.Parse(Console.ReadLine());
string cName = "";
foreach (var cat in iCat)
{
if (cat.Key == iName)
{
cName = cat.Value;
break;
}
}
int cPercentage = 0;
foreach (var c in CGR)
{
if (c.Key == cName)
{
cPercentage = c.Value;
break;
}
}
double fPrice = iQnt * (iRate + iRate * cPercentage / 100.0);
string output = "*******************************************\n" +
"Billing Details for " + iName + ":\n" +
"*******************************************\n" +
"Quantity: " + iQnt +
"\nPrice per unit: " + iRate +
"\nFinal rate: " + fPrice;
Console.WriteLine(output);
Console.WriteLine("\n*********************************\n");
}
}
}

Were you able to understand the code? After a few minutes of repeated reading, you could.

It’s difficult to know the meaning of the variables like CGR, iCat, iName, iQnt, cPercentage, fPrice and so on?

It creates a complicated mapping in your brain. You need to scroll up and down to understand the intention of everything, isn’t it?

Now let’s see the same code after giving little better names.

using System;
using System.Collections.Generic;
namespace NMart.Billing
{
internal class NMartStore
{
private static Dictionary<string, int> CategoryGstRatesInPercentage = new Dictionary<string, int>();
private static Dictionary<string, string> ItemsInCategory = new Dictionary<string, string>();
private static void Main()
{
CategoryGstRatesInPercentage.Add("Food-grains", 0);
CategoryGstRatesInPercentage.Add("Furniture", 5);
CategoryGstRatesInPercentage.Add("Electronics", 18);
CategoryGstRatesInPercentage.Add("Cosmetics", 28);
ItemsInCategory.Add("Rice", "Food-grains");
ItemsInCategory.Add("Wheat", "Food-grains");
ItemsInCategory.Add("Sofa", "Furniture");
ItemsInCategory.Add("Chairs", "Furniture");
ItemsInCategory.Add("TV", "Electronics");
ItemsInCategory.Add("Mobile", "Electronics");
ItemsInCategory.Add("Shampoo", "Cosmetics");
ItemsInCategory.Add("Perfume", "Cosmetics");
Console.WriteLine("Welcome to NMart store");
Console.WriteLine("***************************");
Console.Write("Enter name of item: ");
string itemName = Console.ReadLine();
Console.Write("Enter quantity of item: ");
int itemQuantity = int.Parse(Console.ReadLine());
Console.Write("Enter rate per product item: ");
int ratePerUnitItem = int.Parse(Console.ReadLine());
string categoryName = "";
foreach (var item in ItemsInCategory)
{
if (item.Key == itemName)
{
categoryName = item.Value;
break;
}
}
int gstPercentageForItem = 0;
foreach (var categoryGstRate in CategoryGstRatesInPercentage)
{
if (categoryGstRate.Key == categoryName)
{
gstPercentageForItem = categoryGstRate.Value;
break;
}
}
double finalPrice = itemQuantity * (ratePerUnitItem + ratePerUnitItem * gstPercentageForItem / 100.0);
string output = "*******************************************\n" +
"Billing Details for " + itemName + ":\n" +
"*******************************************\n" +
"Quantity: " + itemQuantity +
"\nPrice per unit: " + ratePerUnitItem +
"\nFinal rate: " + finalPrice;
Console.WriteLine(output);
Console.WriteLine("\n*********************************\n");
}
}
}

Do you see how simple it becomes as soon as you write meaningful names?itemName, itemQuantity, ratePerUnitItem, gstPercentageForItem, finalPrice and so on. Now the variables are conveying their intent, better than the first snippet? (I agree I could use a better data structure or use LINQ. However, I am simulating what students had written. They used different programming languages too. This code is for demo purpose.)

Now, you don’t need to scroll up and down to understand the intention of the variables. This is an impact of giving meaningful names to variables.

Don’t use abbreviations for variable names

Hence, we should spend little more time on giving meaningful names to variables (same thing is applicable to the class, namespace, methods, and so on) so that when you or maintenance developer revisits your code to fix a bug or add a feature, she/he could do it very easily. It should be flawless. Avoid using abbreviations while giving names to variables throughout the project. It would be a nightmare to maintain such code.

Dattatray Kale

Creating reliable “Objects”

Let’s start of with a piece of code.

We have a Main( ) method, which creates an object of a Circle class, sets the radius and then calculates the area of the circle. Let’s have a look at the circle class. It has a simple implementation to calculate the area of circle.

The above code looks simple and it works. Are there any flaws in it? Let’s examine.

What if the consumer of the Circle class, does not set the radius correctly? For example, like this,

circle.Radius = Convert.ToInt32("abc");

The call to CalculateArea( ) would fail, because the input cannot be parsed into an integer. By design, the Circle class  is allowing other classes to control the Radius property. The consuming classes, may set unacceptable values for the Radius property. This leads to unpredictable results from the CalculateArea( ) method. We can do a validation check for the Radius property either in it’s setter or the CalculateArea( ) method. That would work. Perhaps that is a natural approach we adopt most of the time.

But let us for a moment take a step back and think more in terms of object orientation. Imagine someone asks you to draw a circle on a piece of paper. Your obvious question would be, “Sure, how big or small do you want it?”. In other words, in order to “construct” a circle, you would need the information about it’s radius.

One more important aspect in this example, is that a Circle is immutable. Once we create circle with a certain radius, we cannot change it. Making the setter for Radius property as public allows the consuming classes to alter it, even after it has been created (Hmm, not good.)

It is therefore always a good idea to create objects in a robust way, when they are being constructed. The radius property is integral to the Circle class and it is the responsibility of Circle class to manage it. The clients just need to provide the details while creating the objects. If the client code provides incorrect information while constructing the object, the class can choose not to go further with its creation, thereby protecting its integrity.

Below would be the correct implementation of Circle class.
The above example was a simple one. We just had one property to deal with. But in reality, we need to deal with classes which have a lot of properties and really complex methods. Ensuring correct values in all properties throughout the lifetime of an object can be a very tedious task. This leads to many potential bugs. It is impossible to predict the ways in which the consumers of the classes would consume them. This leads to responsibilities scattered all over the place and classes that are strongly dependent on each other.

In addition, once we make a property as public, it is very difficult to make it private again, because by the time you realize that you need to do that, there may be a number of places, it has already been consumed and used in different ways.

As a thumb rule, try to restrict the access to all the properties in the class when you write it for the first time. This will allows you to encapsulate the class properties and the code to manage them inside the class itself. If you need to make any property as protected, internal or public, think and try to find for a strong need and justification for doing so.

 

Fail as early as possible

The inspiration behind this blog post is, of course, the Clean code book.

Sometimes, people choose to use unnecessary deep conditionals.

They choose to return error/exception from else part of the deeply nested conditionals. It hampers the readability.

We will consider the use case of register user which requires three mandatory fields: Name, Email, and Password. (Let’s keep it simple.)

Let me walk you through a code that was created to showcase the problem of deep nesting. This was a part of the method “RegisterUser” that accept user object.

RegisterUser(User user) routine and User has Name, Email and Password fields.

This code snippet creates a little-complicated decision matrix in your head, especially for the Else part.

When User’s Name is NOT empty && User’s email is NOT empty && User’s password is NOT empty then store user and also send a notification for that user.

Whereas Else part was more complicated and may read like:

  • When User’s name is empty then throw an exception for the name is required.
  • When User’s name is NOT empty but user’s email is empty then throw an exception for the email is required.
  • When User’s name is NOT empty and the user’s email is NOT empty but the user’s password is empty then throw an exception for the password is required.

To make it simple, use guard clause to fail early or fail fast. All if clause in below snippet is guard clause.

Now, this one reads like

  • Throw exception for the name as soon as the user’s name is empty.
  • Throw exception for the email as soon as the user’s email is empty.
  • Throw exception for the password as soon as the user’s password is empty.
  • Otherwise, store user and send a notification to the user.

Thus, the fail-fast/fail early would make the code simpler.

Guard clause in such scenarios helps us to improve understandability.

The end result:

Introduce explaining variable

While doing pair programming with developers, I have experienced that some of them are hesitant to introduce the explaining variable. They worry more about other devs thinking about that extra variable.

Will it be redundant to have an extra variable? Well, let’s see:

Before

In the above code snippet (…even though it’s a simple one), checking file size against an arbitrary number does not convey any business rule clearly. You can’t predict easily about the meaning of ‘10000000’ or size whether it is in Bits, Byte, KB, MB or something else. One who visits this snippet needs to spend some time to figure out the intent.

Well, let’s introduce explaining variable and see if it helps.

Better way:

I know, I have increased the line of code here. However, don’t you think the introduction of explaining variable made it expressive and understandable?

Now it reads like:

When file size exceeds maximum allowed size (10000000 Bytes!), then just exit!

Another example:

Before

PHP code doing some matching on phone.

It’s really difficult for me when I read it the first time.

Better way:

Divided into multiple parts so that it could express itself very easily.

Now at least, you know that there is a regular expression for valid phone number pattern. It is used to determine if the phone number is valid or not and take appropriate action.

Introduce the explaining variable to convey your intent. This is one of the ways to express yourself via code.


Why giving meaningful names to things in programming is important

Everything in this world has meaningful names. It helps our brain to understand and remember the things very easily. It means less mental mapping.

Let us take real-world examples of meaningful names given to things like hand wash (of course, you don’t use it to wash your face), face wash, shaving cream, aftershave and so on.

Similarly, in the programming, everything needs to have a meaningful name because we as a developer spend our most of the time reading the code than we do writing it.

As a developer, our daily job is to give a solution to the problem at hand.

To solve the problem at hand, we sometimes create a project, an assembly, a namespace,  directories, class files, classes, fields, methods, other members, UI elements, their translation files, translation keys and so on.

All of these things should have the meaningful name so that they convey their intent and you realize what they are doing by just looking at their name.

Perhaps they answer all the big questions that you or future maintenance developers would have.

Let’s go through the following code snippet. It is written in js.

  downld(recs){ 
        const cols = ['Name', 'Age', 'Gender'];
        let d = `data:text/csv;charset=utf-8,${cols.join(',')}rn`;
        recs.each((r) => {
            const row = [
                r.get('name'),
                r.get('age'),
                r.get('gender')
            ].join(",");

            d += `${row}rn`;
        });
        const u = encodeURI(d);
        const lnk = document.createElement("a");
        lnk.setAttribute("href", u);
        lnk.setAttribute("download", "users.csv");
        document.body.appendChild(lnk);
        lnk.click();
  }

I hope you understood what it is doing.

The above example shows that a simple code snippet could be difficult to understand when meaningful names are NOT given to variables, functions and so on.

Now think about a project that has 100+ classes and thousands of lines.

Don’t you think it will be painful for the maintenance developer as well as an original developer to read it again in the future?

This could have been easy to understand when good names would have been given.

Let us try to give the meaningful name in the code snippet.

  downloadAsCsv(users){ 
        const columnNames = ['Name', 'Age', 'Gender'];
        const csvHeader = `data:text/csv;charset=utf-8,${columnNames.join(',')}rn`;
        let csvDataRows = "";
        const newlineAndCarriageReturn = `rn`;
        users.each((user) => {
            const currentRow = [
                user.get('name'),
                user.get('age'),
                user.get('gender')
            ].join(",");

            csvDataRows += currentRow + newlineAndCarriageReturn;
        });

        const csvFileContent = csvHeader + csvDataRows;
        const encodedUri = encodeURI(csvFileContent);
        const newlink = document.createElement("a");
        newlink.setAttribute("href", encodedUri);
        newlink.setAttribute("download", "users.csv");
        document.body.appendChild(newlink);
        newlink.click();
  }

Now, do you think it’s a little better than the previous version?

The good names encourage you to write clean and good code. You can go one step ahead and split above function.

    downloadAsCsv(users){
        const csvFileContent = this.generateCsvContent(users);
        const encodedUri = encodeURI(csvFileContent);
        const newlink = document.createElement("a");
        newlink.setAttribute("href", encodedUri);
        newlink.setAttribute("download", "users.csv");
        document.body.appendChild(newlink);
        newlink.click();
    }

    generateCsvContent(users) {
        const columnNames = ['Name', 'Age', 'Gender'];
        const csvHeader = `data:text/csv;charset=utf-8,${columnNames.join(',')}rn`;
        let csvDataRows = "";
        const newlineAndCarriageReturn = `rn`;
        users.each((user) => {
            const currentRow = [
                user.get('name'),
                user.get('age'),
                user.get('gender')
            ].join(",");

            csvDataRows += currentRow + newlineAndCarriageReturn;
        });

        return csvHeader + csvDataRows;
    }

I hope you agree with me that meaningful names will make everyone (including you) more than happy after reading it.

In case of methods, when you failed to give a name to a method, you may realize it’s doing more than one thing. It is really helping you.

Even when you are able to give a meaningful name to a method, you may realize that the method doesn’t belong to this class.

I often realized that this practice of giving meaningful names helped me a lot about understanding the solutions, separations of concerns and even responsibility of the classes, methods, assembiles and so on.

Suppose your boss comes to you and asks you to fix value against gender column in generated CSV (from above example) on priority. Instead of debugging and finding the location of the code, just ask IDE to show you “generateCsvContent” routine.

Thus, help your IDE to help you by giving meaningful names to everything in programming so that you can figure out everything very easily.

Consequently, giving meaningful names will help you in some way or other.

When you revisit your code after 4 hours or 4 weeks, or 4 years, and you think you could give more meaningful names now then

DO NOT hesitate to rename it if it’s not breaking anything and has less impact on other features.

REMEMBER!

https://blog.beingcraftsman.com/datta/