This post refactors a piece of code written in the style commonly found in ServiceNow to demonstrate how clean code can lead to decreased interpretation times. It will first show the original code sample, then what it can look like refactored with clean code ideas.

Original Sample

(function process ( /*RESTAPIRequest*/ request, /*RESTAPIResponse*/ response) {

    // implement resource here
    var reqNumber = request.pathParams.number;
    var requestBody = request.body;
    var requestData = requestBody.data;
    var state = requestData.state;
    var errorMessage = requestData.error;


    var body = {};
    var grSCReq = new GlideRecordSecure('sc_req_item');
    grSCReq.addQuery('number', reqNumber);
    grSCReq.query();


    if (grSCReq.next()) {
        grSCReq.state = state;

        if (null != errorMessage && errorMessage.length > 0) {
            grSCReq.comments = errorMessage;
            grSCReq.variables.error_message = errorMessage;
        } else {
            grSCReq.comments = Comments.getPrefixByState(grSCReq) +
                '\n' + Comments.format(requestData.comments);
        }
        grSCReq.update();
        response.setBody(body);
    } else {
        response.setBody(body);
    }

})(request, response);

The code above is relatively simple yet, how long did it take you to interpret what was going on? How sure were you that what was clearly understood? How long will it take the next person to understand the very same lines? How long will it take you to read the very same lines months down the line? Simple code doesn't necessarily mean easily and clearly understood code. Time in business is translated to cost. The longer it takes to read code, the more costly it becomes.

Refactoring #1

(function process ( /*RESTAPIRequest*/ request, /*RESTAPIResponse*/ response) {

    var ritm = getRequestedItem(request);

    hasRecord(ritm) && addResponseCommentsOrError(request, ritm);


    response.setBody({});

})(request, response);

Refactoring #2

(function process ( /*RESTAPIRequest*/ request, /*RESTAPIResponse*/ response) {
    var ritm = getRequestedItem(request);

    if (foundRecord(ritm)) {
        addResponseCommentsOrError(request, ritm);

    }

    response.setBody({});

})(request, response);

Refactoring #3

(function process ( /*RESTAPIRequest*/ request, /*RESTAPIResponse*/ response) {

    HttpScReqItem.addResponseCommentsOrErrors(request);

    response.setBody({});

})(request, response);

The refactoring above moves low level code (algorithms that carry out work) into named functions that give the work being carried out human meaning. The functions from refactoring #1 and #2 above, although not method calls or displayed above, belong in a library. The functions in that lib can then be reused, and themselves refactored to become single purpose sharable functions. The need to rewrite boilerplate code as seen in the original snippet will eventually be minimal.

The functions made from refactoring #1 and #2 could be as follows.

function hasRecord (gr) {
    return gr.next();

}


function getRequestedItem (request) {
    var gr = new GlideRecordSecure('sc_req_item');
    gr.addQuery('number', request.pathParams.number);
    gr.query();

    return gr;

}


function addResponseCommentsOrError (request, ritm) {
    var data = request.body.data;

    return hasErrorResponse(data) ?
        showErrorInRequestedItem(data, ritm) :
        addCommentToRequestedItem(data, ritm);

}


function showErrorInRequestedItem (data, gr) {
    gr.comments = data.error;
    gr.variables.error_message = data.error;

    gr.update();

    return gr;
}


function addCommentToRequestedItem (comment, gr) {
    var comments = Comments.getPrefixByState(gr) +
        '\n' + Comments.format(comment);

    gr.comments = comments;
    gs.update();

    return gr;
}


function hasError (requestData) {
    return requestData.message != null && requesteData.message.length > 0;

}

The goal was to break down the original code into independent units of work. Each in charge of a single thought, and named accordingly.

When a reader comes into an initial line of code, it should be clean, nothing more than the initial thought that starts the process. If there is a need to see more, then follow that thought to the next function, and so on until the search that brought her there is satisfied. Once the interest is fulfilled, the search is abandoned. And key, is that each function should be a single unit for work trivially understood; and not until reaching the lowest possible level, those really technical details found. Pretty much like a search engine result where the reader goes into the results of interest...

happy snowing....

Labels: