Every piece of knowledge or logic must have a single, unambiguous representation within a system

Javier Arroyo · ‎09-08-2019

DRY (Don't Repeat Yourself)

DRY is a programming principle that states "every piece of knowledge, or logic must have a single, unambiguous representation within a system". The purpose of this post is not really to go in length about DRY, as a web search for 'DRY principle' will yield better written posts than can I. The objective, instead, is to explain through code examples how code readability is affected by it.

Sample One:

var Human = function Human(Person) {

    var someone = Object.create(Person);

    if (Person && Person.hasOwnProperty('name') && Person.name === undefined || Person.name === null) {
        someone.anonymous = function anonymous (something) {
            return something || 'I do not have a name';

        };
    }

    return someone;
};

Sample Two:

var Robot = function Robot(name) {
    var bot = {};

    if (name !== undefined && name !== null) {
        bot.name = name;
        bot.move = function move() {
            return 'I\'m moving';

        };
    }

    return bot;
};

Both samples above violate the DRY principle in quite subtle ways. In the few lines of code, the scripts already repeat null checks; furthermore, the first one checks for a property of an object (name) for null. The basic null check, as trivial as it is, can proliferate throughout a code base cluttering code and increasing interpretations times. That means a larger, more complex, less maintainable and less readable code base.

name !== undefined && name !== null 
//while familiarity makes this line 'simple' to read, it's clarity can be greatly improved

To dispose of any familiarity bias when reading, or any hint of ambiguity, the null check logic can be wrapped inside a function (single unambiguous place). Then used throughout the code base. An example of this practice is the OOTB globally scoped function isNil().

For scoped applications, this may be addressed by creating utility functions named with the exact meaning of our intention.

Utility Functions

function isNothing(value) {
    return value === null || value === undefined;

};

function isInObject(obj, propertyName) {
    return obj && obj.hasOwnPropertyName(propertyName);

};

function isInObjectAndIsNothing(obj, propertyName) {
    return isInObject(obj, propertyName) && isNothing(obj[propertyName]);

};

The above functions will replace null checks throughout the code base. If null check needed to be updated, then it should be done in one function, and no where else. In addition to replacing familiarity biases by explicitly expressing intentions, the functions decreases interpretation times, and help build a more concise code base. A smaller code base can lead to less bugs as code surface decreases and, well, cost.

Updated Code

var Human = function Human(Person) {

    var someone = Object.create(Person);

    if ( isInObjectAndIsNothing(Person, 'name') ) {
        someone.anonymous = function anonymous(something) {
            return something || 'I do not have a name';

        };
    }

    return someone;
};

var Robot = function Robot(name) {
    var bot = {};

    if (!isNothing(name)) {
        bot.name = name;
        bot.move = function move() {
            return 'I\'m moving';

        };
    }

    return bot;
};

isNothing
//vs
value === null || value === undefined

The utility functions make the updated scripts more explicit for a human to read. The need to interpret null checks is decreased by stating what we meant to do, check if something isNothing

In all, the goal is to use DRY as part of a repertoire to communicate more eloquently our thoughts when coding. Code should be seen as well written thoughts for readers.

Other places not as obvious where this might be used are GlideRecords and try/catch.

Daniel Draes · ‎09-10-2019

Nice article, and I admit that I have not heard about DRY principle - while for sure this is something every developer should have in his backpack anyway.

Wrt. to your example above, not to sure if this is a great example. Might also be that my developer heart is looking at this differently. I personally would challange that

    if ( isInObjectAndIsNothing(Person, 'name') )

is harder to read than

    if (Person && Person.hasOwnProperty('name') && isNil(Person.name))

Again, this might just be me as I would ask myself 'what the heck is this isInObjectAndIsNothing-function doing?

Javier Arroyo · ‎09-10-2019

Thanks Fogg for the input.

Function names are entirely up to the author. Whatever one thinks describes best. Perhaps, authoring it as objectHasOwnPropertyAndIsNothing would be more meaningful?

But, as far as readability, change the name to something you like. Walk out to the street and ask each passerby to read:

 if (Person && Person.hasOwnProperty('name') && isNil(Person.name))

then read

if (objectHasOwnPropertyAndIsNothing(Person,'name'))

Your proposed challenge comes backed with programming experience and familiarity. It now asks that the reader have a level of JavaScript and familiarity that is comparable to yours. The proposal readily reduces cognition times in that the three conditions must be individually processed and understood. In addition, the behavior of the logical operand && has to be identified and understood (identify it as an && and not an ||, and why it would work). All of that processing always happens every time the conditions are read, regardless familiarity.

How would the passer by test fair? Would they be able to convey more meaning from words, or from figuring out what a three condition, logical and, using isNil() (what is isNil?) with (why is Person all alone)?

The single most important factor in understanding is familiarity. Humans simply understand words far faster than they can interpret topic specific conditions. You might have noticed yourself that words can be missing letters, and sentences full words, yet the processing time is the same. When coding, familiarity biases are a big deal. Even for the name I chose, I figured that 1. everyone should have seen the function isInObject, 2. know that all functions checking for object properties have an arity of two. 3. what isNothing means and that it is a corner stone function in null checks. I presumed a level of familiarity that, while readily understood to me, might not have conveyed it as well as it could have.

Daniel Draes · ‎09-10-2019

As for the familiarity - this is understood and accepted. I would challenge how many of the passer-bys would need to understand this. Thinking about a need-to-know / understand basis sets some context. I cannot expect everyone on the street to speak the same language and even if, not everone would now what a 10 mm wrench is.

I get your points and agree with the principle - just reiterarig 🙂

I was challenging to a certain extend your example. I am a big fan of bringing code into smaller manageable blocks which can be reused. The whole idea of object oriented programing is based on that - so nothing new really. In ServiceNow though we are not fully object oriented, and maybe therefor we see more of the traditional spaghetti code. On the other side we want to have lesser skilled / trained (no-code, lo-code) people being able to create great apps.

Daniel Draes · ‎09-10-2019

As for the familiarity - this is understood and accepted. I would challenge how many of the passer-bys would need to understand this. Thinking about a need-to-know / understand basis sets some context. I cannot expect everyone on the street to speak the same language and even if, not everone would now what a 10 mm wrench is.

I get your points and agree with the principle - just reiterarig 🙂

I was challenging to a certain extend your example. I am a big fan of bringing code into smaller manageable blocks which can be reused. The whole idea of object oriented programing is based on that - so nothing new really. In ServiceNow though we are not fully object oriented, and maybe therefor we see more of the traditional spaghetti code. On the other side we want to have lesser skilled / trained (no-code, lo-code) people being able to create great apps.

Javier Arroyo · ‎09-10-2019

Got it :-). it was a fine rebuttal.

My coding expectations are that even people who don't know squat about programming can surmise something from my code; at least the entry points and high level classes such as workers (it's certainly a work in progress). The abstractions, well, now that is where it gets technical.

My daughters (11 and 😎 act as that guinea pig. If they can talk reasonably about what they read, I stop, otherwise it gets rewritten. I do a lot of rewrites 🙂

You've touched upon quite the sore topic for me: Spaghetti code (and I've written my share)... but more precisely, OOTB code. Do you have any thoughts as to why it's so bad. Even procedural languages are better structured and formulated. To me, whom ever was first tasked with coding JavaScript 1. wasn't a javascript programmer, 2. wasn't theoretically capable, 3. the language itself back in the day. 4. the platform mandated it early on and has stayed the same since.

Javier Arroyo · ‎09-11-2019

Example of a lack of readability from OOTB ScriptInclude ProjectTaskActualDates

 deriveCloseDates: function () {
        if (this.isActualEndDateGiven()) {
            if (this.isActualStartDateGiven()) {
                if ((this.task.allow_dates_outside_schedule == 'true') || (this.task.allow_dates_outside_schedule == "1"))
                    var resp = this.util.RecalculateDurationForOutsideSchedule(this.task.actual_start_date, this.task.actual_end_date);
                else {
                    var resp = this.util.RecalculateDurationFromEndDatePerSchedule(this.task.actual_start_date.getDisplayValueInternal(), this.task.actual_end_date.getDisplayValueInternal(), this.task.schedule_id);
                    resp = new JSON().decode(resp);
                }

                return this.sendSuccess(this.task.actual_start_date, this.task.actual_end_date, resp.duration);
            } else {
                var resp = this.util.RecalculateStartDateFromAbsoluteDurationPerSchedule(this.task.actual_end_date, this.task_duration, this.task.schedule_id);
                resp = new JSON().decode(resp);
                if (resp.status == 'success') {
                    var asd = new GlideDateTime();
                    asd.setDisplayValue(resp.date);
                    return this.sendSuccess(asd, this.task.actual_end_date, this.task_duration);
                } else
                    return this.sendError(resp.message);
            }
        } else {
            if (this.isActualStartDateGiven()) {
                var resp = this.util.RecalculateEndDateFromAbsoluteDurationPerSchedule(this.task.actual_start_date.getDisplayValueInternal(), this.task_duration.getValue(), this.task.schedule_id);
                resp = new JSON().decode(resp);
                if (resp.status == 'success') {
                    var actualEndDate = new GlideDateTime();
                    actualEndDate.setDisplayValue(resp.date);
                    return this.sendSuccess(this.task.actual_start_date, actualEndDate, this.task_duration);
                } else
                    return this.sendError(resp.message);
            } else {
                return this.sendSuccess(this.task.planned_start_date, this.task.planned_end_date, this.task_duration);
            }
        }
    }

Daniel Draes · ‎09-14-2019

I do agree that this is hard to digest 🙂

On the other side, not sure if the names of the functions would help - I would appreciate a comment secion before the code with one/two lines on what the intent of it is. - Which in itself is good practice, comment on what you do.

Daniel Draes · ‎09-14-2019

I do agree that this is hard to digest 🙂

On the other side, not sure if the names of the functions would help - I would appreciate a comment secion before the code with one/two lines on what the intent of it is. - Which in itself is good practice, comment on what you do.

Javier Arroyo · ‎09-15-2019

If readability is seen as the amount of time required to read and comprehend without ambiguity, I fail to understand how comments at the top of a function explaining what is happening in the code, decrease the complexity or shed any light into how the logic has been structured, let alone decrease the amount of time taken to comprehend it.

Comments task the reader with reading comments, still reading the code, then interpreting that the comments and code match. If they don't, or the code isn't clear, that's added time discerning ambiguity. This means that concentration has been split between what do the comments say, the readers recollection, and how the code is actually doing what the comments say. The single most pervasive practice in comments is out-dated comments, which is partly the reason self-documenting code became a thing.

Comments when used properly are solely used to increase clarity, rather than decrease readabilty. The practice of self-documenting code states that there should be no freehand writing to code distinctively clear. Otherwise, comments for the sake of comments because it's "a good practice" hinder code. In addition, what are the effects of the added verbosity? Say a function that was 40 lines long, has been commented, not only at the top but inline to "help the reader". It will likely be 10 to 20 lines longer, likely leading to scrolling up and down when the reader questions recollection, or needs to re-read another section.

There is a plethora of practices in that sample that no comments will help increase readabilty. It needs to be entirely rewritten. As a reader, I don't care to know what a code is doing with comments, I want to only read code that tells me exactly what its doing. That can only happen by completely refactoring that code.

Example:

var disableVIPUserFromSensitiveSystems = function disableVIPUserFromSensitiveSystems (sysId) {
    var user = db('sys_user').retrieve(sysId);
    var sucess = DisableUserProcess.fromSensitiveSystems(user);

    sucess 
        ? NotifyHROfDisabledVIPUser(user) 
        : NotifyHROfFurtherManualDisablingProcess(user)

    return sucess;

};

//vs


var disableVIPUser = function disableVIPUser (sysId) {
    var gr = new GlideRecord('user');
    gr.get(sysId);

    var success;
     
    if (gr.isValidRecord()) {
        //how big will this code be if the same practices used
        //for the sample code were used here for:
        //1. fetch system list from which to disable users
        //2. build HTTP Requests to disable user for each system
        //3. Very that they are all successful

        if (sucess) {
            //how big will this be to build and gather information
            //to email a comprehensive list of what HR expects to
            //see when a VIP user is disabled? 
        } else {
            //how big will this be to notify HR of the manual work
            //still left to do?
        }

    } 

    return success;
};

How much of help are comments to the first script?
What should comments say? "This function disables VIP users by disabling them from sensitive systems then notifying hr of success or failure" And once he's done, how long will it take him to understand the same from reading the code? Why make the reader read twice what can be said and understood once?

In the second example how much clearer will comments be when the reader still has to read another 80 lines of code?

What I am saying about comments is that instead of telling the reader what code does with comments, to actually write what it does with code, rather than coding how it should be done. The only time comments add clarity is when code takes an unexpectedly complexity where the thoughts of the author upon writing it are needed to understand it.

Well written code should be small, precise, nearly and often times, void of comments.

Now note, if the issue with reading code is grammar, the it is entirely up to the author to what he feels is easier to read for his staff. For me, I expect my staff to fully versed with JavaScript, this is done for their benefit. I want them to be as good as they can be with JavaScript. If the expectation of them is to be mediocre, they'll never move on the better things. It's like a third grader complaining that the words in college physics courses don't mean anything.

With a better structure and actual legible function names, that illegible piece of could would be trivial to comprehend.

Mwatkins · ‎08-19-2020

Love the isNothing() example. I see that one done wrong a lot.

value === null || value === undefined

FYI - I just saw something for the first time the other day (or maybe I just forgot). The JavaScript String "null" when sent in to GlideRecord.addQuery(field, value) gets translated to IS NULL on the database.

So, depending on requirements, isNothing(value) might need to cover

value === null || value === undefined || value === "null"

Maybe covering empty String "" might be part of requirements too.

ServiceNow does have a couple isNothing type checks, GlideRecord/GlideElement.nil() and JSUtil.nil() - they cover typeof "undefined", null and empty string but not the literal string "null"

Javier Arroyo · ‎08-19-2020

isNothing checks for "no value". From that point of view the check for string "null" would alter its meaning. Also would differ in implementation from other functional libraries, possibly producing unwanted behavior for unsuspecting coders.

It's one of the reasons I refrain from using JSUtil (JSUtil.nil()): misleading name.

Still, how can isNothing be ServiceNowfied?

It might just be the same as JSUtil.nil with a different name.

From a functional perspective it would be an entry point function with something like

function isEmptyString (value) {
   return value === '';

}

function isNilString (value) {
   return value === 'null';

}

function isNothingOrEmpty (value) {
   return isNothing(value) || isEmptyString(value);

}

function isNothingOrEmptyString (value) {
   return isNothing(value) || isEmptyString(value) || isNilString(value);

}

Seems okay readable and trivial to interpret as to where the reader doesn't guess behavior.

In that same token but somewhat not so: A truthy/falsey check:

function identity (value) {
  return value;

}