2
\$\begingroup\$

I want to write a generic parser which takes a value, and type and returns label of given value instead.

Currently, this is my code:

import a from "../constants/a"
import b from "../constants/b"
import c from "../constants/c"

const find = (value, fromArray) => fromArray.find((term) => term.value === value);
const getLabel = (of, fromArray) => {
    const value = find(of, fromArray);
    return (value && value.label) || 'No info';
};


export default (value, type) => {
    switch (type) {
        case 'paymentMethod':
            return getLabel(value, paymentMethods);
        case 'paymentTerm':
            return getLabel(value, paymentTerms);
        case 'dateType':
            return getLabel(value, dateTypes);
        default:
            return 'No info';
    }
}

What would be more ES6, faster and concise way to achieve this?

\$\endgroup\$
2
\$\begingroup\$

Granularity

All code can be said to have a level of granularity. This is a measure of how many functions it contains. Low granularity has fewer larger functions that has benefits of faster run-times at the expense of readability, while high granularity has many smaller functions, trading off performance for more readable and maintainable code.

Just how granular to make some code is something that comes from experience as there are no hard and fast rules to go by. However there is a general rule of thumb that increasing granularity should always reduce code complexity and source code size.

Too granular

Your code is too granular,

  • It is bloated with the source required only to define functions, call and pass arguments.
  • It adds un-needed complexity with arguments transforming the abstraction implied by naming from one value to another of and then back again to value.
  • It has forced you to repeat code.
  • It contains a function that is only an alias for existing functions find is an alias for Array.find and provides no additional processing.

The function getLabel processes the result of find returning "No info" for undefined results. But this is repeated in the calling function.

Reducing granularity to improve code

Reducing granularity will not always improve code, well I should say very seldom and only in the case where the code is already too granulated.

Your module can be done in a single function

export default (value, type) => {
    var array, result;
    switch (type) {
        case 'paymentMethod':
            array = paymentMethods;
            break;
        case 'paymentTerm':
            array = paymentTerms;
            break;
        case 'dateType':
            array = dateTypes;
    }
    if (array) { result = array.find(item => item.value === value) }
    return result && result.label || "No info"
}

Or as there are only a few cases in the switch use if else

export default (value, type) => {
    var array, result;
    if(type === "paymentMethod") { array = paymentMethods }
    else if(type === "paymentTerm") { array = paymentTerms }
    else if(type === "dateType") { array = dateTypes }

    if (array) { result = array.find(item => item.value === value) }
    return result && result.label || "No info"
}

This can be even further reduced by using type to index the array via bracket notation, replacing the unwieldy switch or if else statements

const types = {paymentMethods, paymentTerms, dateTypes};
export default (value, type) => {
    var result;    
    const name = type + "s";
    if (types[name]) { result = types[name].find(item => item.value === value) }
    return result && result.label || "No info";
}

Or if the names are not always pluralized with an s.

const types = {paymentMethod:paymentMethods, paymentTerm:paymentTerms, dateType:dateTypes};
export default (value, type) => {
    var result;    
    if (types[type]) { result = types[type].find(item => item.value === value) }
    return result && result.label || "No info";
}

Your questions

What would be more ES6, faster and concise way to achieve this?

  • More ES6? Not really, the ES6 - eness of the code has not changed.
  • Faster? Yes there is less code to execute and few function states to create, push, and pop from the call stack.
  • Concise? Yes in my view it is much more concise. From 17 lines and three functions to 6 lines, one function, and no repeated code.
\$\endgroup\$
  • \$\begingroup\$ In your last iteration - anything with the type paymentTerm won't work as the variable is paymentTerms \$\endgroup\$ – Gerrit0 Aug 25 '18 at 3:48
  • \$\begingroup\$ @Gerrit0 Oh dear I missed that will fix now. \$\endgroup\$ – Blindman67 Aug 25 '18 at 6:29
  • \$\begingroup\$ @Blindman67 thanks for explanation, code also looks great! \$\endgroup\$ – Nuruddin Iminokhunov Aug 25 '18 at 16:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.