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.


Image source: https://quotefancy.com/quote/1469345/Billy-Wilder-Don-t-be-too-clever-for-an-audience-Make-it-obvious-Make-the-subtleties
https://blog.beingcraftsman.com/datta/

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:

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.

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/