Preventing The Misuse Of Variables

One of the most annoying bugs in programming must be having all the right data in your variables, but just passing the wrong ones to a method. Like many other languages, Java and C++ are strictly typed. This has the advantage that the compiler can show you an error while compiling that you have misused a variable. By doing this it becomes impossible to have a string and pass it to a method where it expects an integer. But this does not solve every problem, and a method that takes multiple arguments of the same type remains a situation to be careful with. So is there a way to can help ourselves avoid making this mistakes?

A first step is knowing what the variables you have really mean. An integer with the name i doesn’t tell you much about the value it is storing. With a variable you can always look back at where you assigned it to get it’s meaning, but this is much harder for fields as the assignment can be much further away from the current code. It is best to avoid all this time consuming work to find out what the variable means and just make your code more readable by having a good name.

A known way to do this is by using Apps Hungarian notation, while the term ‘Hungarian notation’ has gotten a bad name in software development the true origin of it makes a lot more sense. Where hungarian notation simple is prepending your fields or variables with a letter indicating the type, apps hungarian notation prepends the variables with a semantic meaningful prefix. The idea behind this naming scheme is to make it visually explicit that something is done wrong. An example of this can be seen here: Making Wrong Code Look Wrong.

But even when you know exactly what all your variables are you can still be puzzled by what order you should pass them to the method. Imagine some very badly designed methods related to showing some basic dots, lines and arrows on a screen:

void createDot(int i, int j, int flag)
void createLine(int i, int j, int u, int v, int flag)
void createArrow(int i, int j, int u, int v, int flag)

If these methods do not have decent documentation explaining the meaning of all these parameters, you either have to guess it or dive into the code and figure it out. A decent assumption can be they represent x-y-coordinates, but perhaps they represent an offset from the upper left corner or the center of the screen is seen as (0,0). Also notice the flag being of the exact same type as our coordinates.

We are faced with multiple problems here. A first improvement would be to either give the parameters meaningful names or have some documentation. Doing this will probably lead to a lot of duplicated documentation related to the semantics of the coordinates and the respective center of the system. An improvement would be to create a Coordinate class, this allows us to move all the documentation of the coordinate system to this class. It will also be much harder to accidentally pass the wrong variable that happens to be an integer as well. Lets look at our new signatures:

void createDot(Coordinate co, int flag)
void createLine(Coordinate co1, Coordinate co2, int flag)
void createArrow(Coordinate startCo, Coordinate stopCo, int flag)

The only method where we can make a mistake is the createArrow, as the createDot method has all different types of parameters and the createLine doesn’t care about the order of the coordinates. Can we do even more effort to avoid these mistakes? We could introduce a new class for this. But lets take a closer look at the problems we solved so far, is the possibility of the bug really gone? While it is true you can’t mistake the flag for a coordinate anymore you will most likely still be able to flip the x and y coordinate as the Coordinate class will look something like this:

Coordinate(x, y)

While the class will be useful in this situation, as it allows for shorter signatures, having it as a return value and just makes conceptually sense. My team lead (with 30 years of experience in software development) showed me to use objects for Id’s instead of string. The code looked something like this:

String locationName = “loc”;
String loadId = “1”;
foo(locationName, loadId);

This was not acceptable for him because of the double string parameters next to each other. He created a LoadId class, which wraps a string, but did not do any attempt to change the id up until the core of the system (which might have been hard because it was persisted to a database with Hibernate). Instead he made the object some kind of factory that allows you to get the LoadId from the string. The resulting code looks like this:

String locationName = “loc”;
LoadId loadId = LoadId.getByString(“1”);
foo(locationName, loadId);

Not only does this change makes nearly no difference with regards to the bugs you can write, as you have to get the LoadId every time you really need it. To decrease the cost of instantiating an object every time you need it, he used a cache inside of the LoadId class. This cache isn’t very safe as it will store every object forever. In this case it will not matter as the LoadId objects are limited in amount and ids, but this could have been a major memory leak if this was not the case.

In my opinion objects should not be used lightly and they should only be used for good Object-Oriented design. Having a class for an id, wrapping either a string, int or long and having no added value makes no sense to me.

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

This site uses Akismet to reduce spam. Learn how your comment data is processed.