Do extracting piece of logic to function improve or reduce code readability?

Question

If there is some code that needs to be implemented across multiple controllers (lets say, 10), e.g.:

// Inside a controller function

var myField = null;
var response = service.callBackend(function(){

    // Other logic

    // starts here
    if (response) {
        myField = { 
             name: response.name,
             value: response.val,
             access: access.READ,
        };
    };
    // ends here

    // Other logic

});

What is a better approach? To create a myField in every controller as an object with the check for response, or extract the logic into separate function?

function helper() {
    return {
        createReadonlyField: function(content) {
            if (!content)
                return null;

            return {
                name: content.name,
                value: content.val,
                access: access.READ
            };
        }
    };    
};

And then in every controller you would call helper function to construct the field:

// Inside a controller function (helper object is passed as Dependency Injection)

var myField = null;
var response = service.callBackend(function(){

    // Other logic

    myField = helper.createReadonlyField(response);

    // Other logic

});

Is the code readability reduced by extracting it to a function?

Answer 1

Several comments:

1. You've created a closure helper to create a relatively simple object, so you could use a simple function instead of a closure. When you use a substantially more complex construct than necessary, it makes the reader wonder why, and this adds to confusion. The following function does the same without the closure.

       function createReadonlyField (content) {
           if (!content)
               return null;
   
           return {
               name: content.name,
               value: content.val,
               access: access.READ
           };
       }
   

However, I appreciate that you may really need a closure in your real code, that the example may just be overly simplified for question asking purposes.

2. Generally speaking, extracting something to a function is useful because you can give it a good name. However, it is not necessary especially if you only use it once . Definitely extract to function if using more than once (keep it DRY). And sometimes even if only once, when it is relatively isolated and deserves its name.

Most importantly, you need to look at the abstractions you're creating for the client/user/fellow programmers/yourself. A single helper function is likely too small as a unit of abstraction.

A good abstraction necessarily is a collection of capabilities that work together and can hide details of implementation, can be passed around as an entity, and can be swapped out with an alternate implementation.

A single function generally can't do that, because usually you then need to know what the other functions are for the other capabilities, and we can't swap them all out together. So, that leaves the client/user having to know more about the underlying details of the abstraction in order to reach the missing capabilities.