On Defense of "Bad" Variable Names: is GR really that bad.
the variable name gr is a perfectly valid name when used within a logically small context. First look at scope and code structure before being fatal about the use of gr. Context means everything. Although I find gr a technically reasonable identifier, I myself don't use it.
----------------------------------------------------------------------------------
This article will look to provide some context for the valid use of gr as variable name.
If you are like me browsing through the community, linkedin, developers.com, slack or discord, following MVPs, etc just to catch some snowflakes, you'll soon discover that there seems to be a high degree of polarity associated with the use of gr as a variable name. The crux of argument against it is somewhat rooted in industry standard programming conventions (some are just lack of subject matter knowledge muddied by experience). It's proponents are mostly on the side of familiarity.
Naming conventions say that identifiers should be logical, unambiguous, reveal function content, larger than 3 characters to help convey meaning and help with readability, etc. Those conventions, however, don't apply in a void, they must be applied within the context in which they are being used.
For example, a less addressed convention states that smaller the scope of a variable (how long it lives), the smaller its name can be. It is meant to account for details that accept a one character variable name as equally suited for the task as a longer one. It means to tells us that context, the lifespan should be considered when naming variables.
It is in that duration that a variable exists that readily reveals when to use GR. If function is 3 or 5 lines long, variable names can shrink as small as a single character. Presuming, of course, that the function is well named. At the level of a short lived variable name, a longer name grants no reasonable or technical advantage over gr. It might even be some sort of non technical win because of familiarity bias.
However, it is in smelly code that gr truly becomes an obstacle. Smelly code means bad structure, scope, format, all of those characteristics that the slightest addition of ambiguity further damages readability and sanity.
For example:
getFieldRequirements: function(sys_class, table, state, oldState) {
var mandatory = "";
var notmandatory = "";
var readonly = "";
var notreadonly = "";
var visible = "";
var notvisible = "";
var isLoad = true;
if (oldState)
isLoad = (state == oldState);
var gr = new GlideRecord(sys_class);
if (isLoad)
gr.addQuery("start_text", "");
else
gr.addQuery("start_text", "").addOrCondition("start_text", oldState);
gr.addQuery("end_text", state);
gr.addQuery("table", table);
gr.addQuery("active", true);
gr.query();
// Combine all field requirements that fit this state transition
while (gr.next()) {
if (gr.mandatory_fields)
mandatory += this._translateFields(gr.mandatory_fields);
if (gr.visible_fields)
visible += this._translateFields(gr.visible_fields);
if (gr.read_only_fields)
readonly += this._translateFields(gr.read_only_fields);
if (gr.not_mandatory)
notmandatory += this._translateFields(gr.not_mandatory);
if (gr.not_visible)
notvisible += this._translateFields(gr.not_visible);
if (gr.not_read_only)
notreadonly += this._translateFields(gr.not_read_only);
}
return {mandatory:mandatory, notmandatory:notmandatory, readonly:readonly, notreadonly:notreadonly,
visible:visible, notvisible:notvisible};
}
There is all sorts of problems with the function above. And, to add to it, the use of gr to the OOTB code block adds further ambiguity. What exactly is the query returning? We all know it can only return a GlideRecord; and, why wasn't gr named logically accordingly to the expected return? The author has chosen to highlight the least important trait of the variable, its type, over informing the reader what is being returned. In clean code the author of the code block is said to be inconsiderate, as he has preferred effort of naming the variable over conveying his intentions. Does it really matter that the query is a GlideRecord when the reader still has to analyze and reason the entire cryptic construct?
That function is the sort of coding that leads to the ultimatum to "never, ever use gr". But, the basic issue here is poor structure and mixture of concerns that create one of those wtf moments (it is said the readability of code is best determined by how many wtf come out when first looking at a function). If this is the type of code that we are used to, then it becomes perfectly understandable why some coders believe not using gr as an identifier can help fend off bugs. And they are right, we are so institutionalized to poor code that we don't even that it's become the acceptable practice, only fixed with superficial adjustments.
The code block isn't asking for better variable names, comments, or formatting to improve it. It's really begging for tender love and care to reduce it to a manageable construct. It's simply doing too much to have, say, gr replaced with activeFieldsByState help it in any meaningful way. The reader still left wondering what is happening.
Reasonable use of gr
function allOpenIncidentsByUser (sysUserId) {
var gr = new GlideRecord('incident');
gr.addEncodedQuery('active=true^caller_id=' + sysUserId);
gr.query();
return gr;
}
In the code fragment above, gr lives for precisely four lines. The function name identifies what the function body is doing clearly enough that there is no need to somehow reiterate part of the function name in the variable. It might make folks like me feel better that the variable is named "logically", such as openedIncidents but, the fact is that it doesn't help or hinder readability of the code. gr works just as fine as a longer name.
If the issue is the lack of readability for gr, then the issue isn't the variable but the structure and architecture. They must change to improve readability. This means a wrapper to GlideRecord to be leveraged in all sorts of ways as to bring code language closer to English grammar. It could then transform to:
function allOpenIncidentsByUser (sysUserId) {
return DBTable('incident').query('active=true^caller_id'= + sysUserId);
}
or
function allOpenIncidentsByUser (sysUserId) {
return SysUser().myOpenedIncidents(sysUserId);
}
Final thoughts
Next time someone expresses some sort of truth about gr try to find out the content in which the argument for, or against exists. The context will make the difference understanding the argument.
So. While I don't use it myself. It's well informed and intended use is as valid as my preference not to use it even in small functions.
https://www.servicenow.com/community/developer-articles/on-defense-of-quot-bad-quot-variable-names-is-gr-really-that-bad/ta-p/2320632