A bit of code smell: Collocating type in variable names

[Javier Arroyo]

This post is about a specific code characteristic that can hinder design quality, while not a problem on it's own, it is part of a larger grouping that has come to be known as Code Smells. 

Collocating, such as prefixing some form of meta-data (i.e type) in a variable name originates from the misunderstanding of Charles Simonyi naming convention Hungarian Notation. The convention, as explained by the inventor says that it is meant to clearly communicate with future readers. Characteristics such as quick identification of possible errors and intent.

The conflict, however, arose into a redundant practice when Microsofts’ lead developer mistook Charles Simonyi -also from MS- scholarly article about variable names. He misinterpreted type in variables to mean identifying the type of the variable within the name, rather than a developer generated identifier to highlight  a specific use case. From there the misunderstood practice became ubiquitous in MS products, quickly spilling into the larger software community. 

From a technical perspective, in strongly typed languages and use of IDEs, collocating variable types is useless. For loosely type languages such as JavaScript, the practice can be debated but, IMHO, only when correctly applied. But, has it's application been properly implemented? The quick answer is that there has hardly been correct usage of Hungarian Notation; 40 years later programmers still abide by the mistaken notion that type in variables means prefixing the type of the Object in the variable name to bring about clarity and intent. 

Despite its well intentions, type prefixed variables are no more than familiarity biases spilling into code and a misplaced commodity that clutters, distracts and calls attention to lesser important details in code. Hungarian notation, at least so what has become known as Systems Hungarian Notation (MS mistaken understanding of the rule), has been considered a bad practice since it fell out of favor with the release of the dotnet in 2002. In that same token, the correct and original usage of Hungarian Notation can be called App Hungarian Notation. This distinction tries to dispel the bad connotation behind a 40 year old mistake.

So, what does commoditizing the wrong item in a variable name mean in the context of Code Smell, or to characteristics that hinder code quality?  It means redundancy, clutter, variable complexity, distraction to readers to the unimportant parts, etc. In a more modern approach to programming such as OO, the successor to Procedural paradigm, followed by the advent of patterns, the use of System Hungarian Notation has further distanced authors from the need to prefix an already redundant value to a variable name. In a structure that asks functions to become single purpose, the need to identify by type doesn't help at all, therefore deemed undesirable since 2002.

Hungarian Notation in ServiceNow

Hungarian notation in ServiceNow is most commonly used when instantiating GlideRecord and ScriptIncludes, tasking the reader to recall a non-functional identifier. 

For example:

function getActiveIncidents () {
   var grIncident = new GlideRecord(‘incident’);
   grIncident.addActiveQuery();
   grIncident.query();

   return grIncident;

}

The use of gr readily clutters and distracts the reader, the code occupies more space than functionally needed, it loses idiomatic expression, calls attention to redundancy, etc. The 4 line piece of code is now considered to smell, even when problem free, it's hindered readability. A 4 line piece of code, in a system in which the only DB access is through GlideRecord, should there be a reason to identify a variable as gr to readily call attention that the line var grIncident = new GlideRecord('incident') is a GlideRecord? From a Code Smell perspective it doesn’t help identify bad code, nor provides functional use, nor helps prevent bugs; it is actually considered smelly code. 

More examples:

var gaIncident = new GlideAggregate(‘incident’);
gaIncident.addAggregate(‘state’);
gaIncident.query();

var gaAjax = new GlideAjax('SomeName');

var grsIncident = new GlideRecordSecure('incident');

var grdsDoer = new GlideDoerSomething();

...

Is it really paramount to collocate gr with the variable name? Has the type in the variable added so much value that overrules idiomatic reading, redundancy, clutter and bloating? Has it helped future readers identify functionality more clearly than a variable name without it? While I think prefixing type in a variable name is of no functional use when improperly applied, that’s for you to decided. Though the consideration should then be: what sort of code structure gains value by prefixing types for enhanced clarity?

Code Smell tells us to never (or nearly never) prefix types in variable names, going as far as categorizing it as code smells within classes or bloater or dispensable. Keep in mind, however, that it doesn’t mean to never use Hungarian Notation as distinctively explained by its inventor, Charles Simonyi: to quickly communicate with future readers programmatic intent. That means there is a decision to be made that will affect functionality at some point, and a quick way to solve it is to name the variable is such a way that it becomes clear something must happen to it.

For example, say an input needs to always morph before it is saved to the database. Hungarian notation is used to stress the fact that some form of sanitation needs to happen. By adding a prefix to the call, it brings to attention that something must happen, and as a reader becomes acquainted with the particular convention, able to identify possible downstream errors.

var unSanUserName = user.getValue(‘name’);
var sanName = doSomethingWithUsanitizedName(unSanUserName);

The prefix of unSan and san readily brings to the forefront possible bad code thereby help to avoid errors. With time, as readers become familiar with the codebase, reading  user.getValue(‘name’) will identify a possible problem: name must always be sanitized. The practice can be made reader friendlier by including a SantizeName wrapper (such as below). There is even the possibility of providing a function to extract and sanitize the name all together, rendering proper Hungarian Notation as redundant:

var sanName =  SantizeName(user.getValue(‘name’));

The goal of Hungarian notation is not to add redundancy because of familiarity biases or to follow mistaken practice but, for it to have a functional/structural meaning that isn’t bound to ambiguity. Variables identified by meta-data aren’t functionally beneficial, instead identify them by their behavior or use: openedIncidents, closedIncidents, newRequestedItems, changeRequestSchedules, changeRequestWindows, currentUserIncidents, stateCategorizedIncidents. Do try to extract all noise that hinders comprehension times.

In the previous GlideRecord example grIncident brings attention to the type yet, fails to identify what about the query. Not only is gr distracting, the variable has an ambiguous name. It causes all the lines of code making up the GlideRecord query to be read to learn that grIncident is active incidents. This logically alludes to active incidents substituting the redundancy of collocating gr and Incident. In ServiceNow, in a one line of code identifier, the reader doesn’t have a functional reason to care that a variable is a GlideRecord, we all know it's the only way to get a record. GlideRecord isn’t going to return another type but GlideRecord. The practice might be debatable if there were multiple DBs where it was paramount to know from what DB and object is coming.

The updated name activeIncidents from grIncident gives more meaning than the type reveals about variable contents and its intent. Gone is the redundant gr, functionally redundant identification of GlideRecord, while left behind is purely the functional worth of the variable. No more bloating, no redundancy, just idiomatic code.

var activeIncidents = new GlideRecord(‘incident’);
activeIncidents.addActiveQuery();
activeIncidents.query();

The goal is to create easy to read code, when something gets in its way, perhaps another approach can be found.

[Badarinarayan B]

I am not so sure.

I am all for getting rid of prefixes/suffixes in code but in ServiceNow + JS - there are different problems. 

For one thing, JS in ServiceNow can have client scripts, server scripts (read controller) and database calls. There is no easy way to identify which's which. For me, when I see a piece of code prefixed or suffixed with 'gr' I know there is query (provided it is followed properly) and my focus would turn to what it's doing, is it needed and can it be optimized. Some of the performance bugs that I have fixed have taught me to look for the dreaded 'gr'.

Secondly, it is not practicable to expect people to name everything properly. Thinking of a good and correct variable name is actually much harder than it seems for any code/statement that does something "substantial". Further, for many, english is not event the native or first language. That's no excuse to name badly, but it does affect the kinds of name we use in code. So, at least with a badly named 'gr', I know it's a query and can be looked into. I think likelihood of ServiceNow developers using 'gr' incorrectly is much less than likelihood of using a misleading name. I might be wrong on that though.

Finally, let's take the example you've mentioned:

activeIncidents might mean activeIncidents from incident table today, but we all know how software changes. Tomorrow, say, if we fetch activeIncidents from a "script" (or some process other than a table query) and not from 'incident' table. So then there are two usages of activeIncidents but both have different meaning, data source and - in all probability - intent.

prefix/suffix 'gr' won't solve this, but if you're using 'gr' all through, you know it's a query. If it's a script (and assuming you follow the 'gr' convention!) you won't prefix/suffix it with 'gr'. That way, I know one gets from table and one from someplace else.

I think ServiceNow + JS combination makes it usage of prefix/suffix rather valuable. I would say it's one way of reducing clutter.

But again, that's me 🙂

[Javier Arroyo]

English names can certainly be a darn to come around for non-native speakers. There was a point where my variables were in spanish :-). I liked them. hehehe But, 15 years ago I would have bought the non-native speaker argument. Today I know it's simply a characteristic that takes a certain level of desire to want to do.  

I once worked in a Japaneses code base. Their rule of thumb, translate your variable and method names. They would then run through a code reviewer looking to improve names; their names were spotless. I, on the other hand, a non english speaker, and before caring to code as a craft, didn't care for naming conventions. I even used spanish because I was better at it. Today, I look back at the Japanese company and commend them from caring enough about their craft to build a process around naming. 

Context is everything. looking at smelly code, like that in SNow, from the prefix/suffix point of view it makes sense to think that gr is really making a difference. But, the issue really isn't with the obstacles solved by hungarian notation, rather how the lack of code structure and the smell of OOTB code seems to be helped by by syntactic sugar. It doesn't matter where Script is located, if it's ambiguous gr won't help any. What if all my client script variables that are Plain JS objects are prefixed with gr (because I dont speak english and I'm having a hardtime naming them)? It's a false safety measure. Even more when using an IDE.

Code in ServiceNow is ambiguous, messy and unreadable. I can not blame new programmers for looking methods to help them weave through the mess.