5
\$\begingroup\$

I'm looking for optimizations and/or improvements. The code's flagship method read takes qty,item in the form of an array of arrays.

pluralizer.read([[2,'orange'],[3,'peach'],[5,'cherry']])

returns string

2 oranges, 3 peaches, and 5 cherries

GitHub

//Revealing Module Pattern (Public & Private) w Public Namespace 'pluralizer'
var pluralizer = (function() {
    var pub = {};
    var r = 'pluralizer.js error';
    var expectedArrayOfArrays = {name:r, message:'Invalid argument.  Expected array of arrays'};
    //creates Array.isArray() if it's not natively available
    if (!Array.isArray) {
        Array.isArray = function(arg) {
            return Object.prototype.toString.call(arg) === '[object Array]';
        };
    }
    if (!String.prototype.endsWith) {
        String.prototype.endsWith = function(searchString, position) {
            var subjectString = this.toString();
            if (typeof position !== 'number' || !isFinite(position) || Math.floor(position) !== position || position > subjectString.length) {
            position = subjectString.length;
            }
            position -= searchString.length;
            var lastIndex = subjectString.indexOf(searchString, position);
            return lastIndex !== -1 && lastIndex === position;
        };
    }
    var irregular = [['child','children'],
        ['die','dice'],
        ['foot','feet'],
        ['goose','geese'],
        ['louse','lice'],
        ['man','men'],
        ['mouse','mice'],
        ['ox','oxen'],
        ['person','people'],
        ['that','those'],
        ['this','these'],
        ['tooth','teeth'],
        ['woman','women']];
    var xExceptions = [['axis','axes'], ['ox','oxen']];
    var fExceptions = [['belief','beliefs'],
        ['chef','chefs'],
        ['chief','chiefs'],
        ['dwarf','dwarfs'],
        ['grief','griefs'],
        ['gulf','gulfs'],
        ['handkerchief','handkerchiefs'],
        ['kerchief','kerchiefs'],
        ['mischief','mischiefs'],
        ['muff','muffs'],
        ['oaf','oafs'],
        ['proof','proofs'],
        ['roof','roofs'],
        ['safe','safes'],
        ['turf','turfs']];
    var feExceptions = [[' safe','safes']];
    var oExceptions = [['albino','albinos'],
        ['armadillo','armadillos'],
        ['auto','autos'],
        ['cameo','cameos'],
        ['cello','cellos'],
        ['combo','combos'],
        ['duo','duos'],
        ['ego','egos'],
        ['folio','folios'],
        ['halo','halos'],
        ['inferno','infernos'],
        ['lasso','lassos'],
        ['memento','mementos'],
        ['memo','memos'],
        ['piano','pianos'],
        ['photo','photos'],
        ['portfolio','portfolios'],
        ['pro','pros'],
        ['silo','silos'],
        ['solo','solos'],
        ['stereo','stereos'],
        ['studio','studios'],
        ['taco','tacos'],
        ['tattoo','tattoos'],
        ['tuxedo','tuxedos'],
        ['typo','typos'],
        ['veto','vetoes'],
        ['video','videos'],
        ['yo','yos'],
        ['zoo','zoos']];
    var usExceptions = [['abacus','abacuses'],
        ['crocus','crocuses'],
        ['genus','genera'],
        ['octopus','octopuses'],
        ['rhombus','rhombuses'],
        ['walrus','walruses']];
    var umExceptions = [['album','albums'], ['stadium','stadiums']];
    var aExceptions = [['agenda','agendas'],    
        ['alfalfa','alfalfas'], 
        ['aurora','auroras'],   
        ['banana','bananas'],   
        ['barracuda','barracudas'], 
        ['cornea','corneas'],   
        ['nova','novas'],   
        ['phobia','phobias']];
    var onExceptions = [['balloon','balloons'], ['carton','cartons']];
    var exExceptions = [['annex','annexes'], 
        ['complex','complexes'], 
        ['duplex','duplexes'], 
        ['hex','hexes'], 
        ['index','indices']];
    var unchanging = ['advice',
        'aircraft',
        'bison',
        'corn',
        'deer',
        'equipment',
        'evidence',
        'fish',
        'gold',
        'information',
        'jewelry',
        'kin',
        'legislation',
        'luck',
        'luggage',
        'moose',
        'music',
        'offspring',
        'sheep',
        'silver',
        'swine',
        'trousers',
        'trout',
        'wheat'];
    var onlyPlurals = ['barracks',
        'bellows',
        'cattle',
        'congratulations',
        'deer',
        'dregs',
        'eyeglasses',
        'gallows',
        'headquarters',
        'mathematics',
        'means',
        'measles',
        'mumps',
        'news',
        'oats',
        'pants',
        'pliers',
        'pajamas',
        'scissors',
        'series',
        'shears',
        'shorts',
        'species',
        'tongs',
        'tweezers',
        'vespers'];
    var doc = document;

    doc.addEventListener("DOMContentLoaded", function(event) {

    });
    pub.help = "Pluralizer.js returns 2 public methods - read and format.  Pluralizer.read expects an array of arrays, each with quantity and item name, e.g. pluralizer.read([[2,'orange'],[3,'peach'],[5,'cherry']]) returns string '2 oranges, 3 peaches, and 5 cherries.'.  Pluralizer.format expects an array with quantity and item name, e.g., pluralizer.format([3,'couch']) returns array '[3, 'couches']'"
    pub.read = function (arr) {
        if(isArrayOfArrays(arr)){
            var count = arr.length;
            var str = '';
            var temp = [];
            switch (count) {
                //if arr has 1 item is 1 apple (no and no commas)
                case 1:
                    temp[0] = pluralizer.format(arr[0]);
                    str = temp[0][0] + ' ' + temp[0][1];
                    break;
                //if arr has 2 items it's 1 apple and 2 oranges (no commas but an and)
                case 2:
                    temp[0] = pluralizer.format(arr[0]);
                    temp[1] = pluralizer.format(arr[1]);
                    str = temp[0][0] + ' ' + temp[0][1] + ' and ' + temp[1][0] + ' ' + temp[1][1];
                    break;
                //if arr has 3 items or more it's 1 apple, 2 oranges, and 3 cherries (the last item has an 'and ' put before it)
                default:
                    // for each item in array output format it and concatentate it to a string
                    var arrayLength = arr.length;
                    for (var i = 0; i < arrayLength; i++) {
                        temp = pluralizer.format(arr[i]);
                        //if this is 2nd last item append with ', and '
                        if (i === arrayLength - 2){
                            str += temp[0] + ' ' + temp[1] + ', and ';
                        }
                        //if this is last item append with '.'
                        else if (i === arrayLength - 1){
                            str += temp[0] + ' ' + temp[1] + '.';
                        }
                        else {
                            str += temp[0] + ' ' + temp[1] + ', ';
                        }
                    }
            }
            return str;
        } else {
            throw expectedArrayOfArrays;
        }
    }

    pub.format = function (arr) {
        //if qty is greater than 1 we need to add s, es, or ies
        var qty = arr[0];
        var str = arr[1];
        if (qty > 1){
            //Word ends in s, x, ch, z, or sh
            if (str.endsWith('s') || str.endsWith('x') || str.endsWith('ch') || str.endsWith('sh') || str.endsWith('z')){
                //look for exceptions first xExceptions
                for (var i = 0; i < xExceptions.length; i++) {
                    if(str === xExceptions[i][0]){
                        return [qty,xExceptions[i][1]];
                    }
                }
                //str = str.substring(0, str.length - 1);
                str = str + 'es';
                return [qty,str];
            }
            // Ending in 'y'
            else if (str.endsWith('y')){
                var s = str.substring(0, str.length - 1);
                // preceded by a vowel
                if (s.endsWith('a') || s.endsWith('e') || s.endsWith('i') || s.endsWith('o') || s.endsWith('u')){
                    str = str + 's';
                    return [qty,str];
                } else {
                    //drop the y and add ies
                    str = s + 'ies';
                    return [qty,str];
                }
            }
            //Ends with 'ff' or 'ffe'
            else if (str.endsWith('ff') || str.endsWith('ffe')){
                str = str + 's';
                return [qty,str];
            }
            //Ends with 'f' (but not 'ff')
            else if (str.endsWith('f')){
                //look for exceptions first fExceptions
                for (var i = 0; i < fExceptions.length; i++) {
                    if(str === fExceptions[i][0]){
                        return [qty,fExceptions[i][1]];
                    }
                }
                //Change the 'f' to 'ves'
                var s = str.substring(0, str.length - 1);
                str = s + 'ves';
                return [qty,str];
            }
            //Ends with 'fe' (but not ffe')
            else if (str.endsWith('fe')){
                //look for exceptions first feExceptions
                for (var i = 0; i < feExceptions.length; i++) {
                    if(str === feExceptions[i][0]){
                        return [qty,feExceptions[i][1]];
                    }
                }
                //Change the 'fe' to 'ves'
                var s = str.substring(0, str.length - 2);
                str = s + 'ves';
                return [qty,str];
            }
            //Ends with 'o'
            else if (str.endsWith('o')){
                //look for exceptions first oExceptions
                for (var i = 0; i < oExceptions.length; i++) {
                    if(str === oExceptions[i][0]){
                        return [qty,oExceptions[i][1]];
                    }
                }
                //Add 'es'
                str = s + 'es';
                return [qty,str];
            }
            //Ends with 'is'
            else if (str.endsWith('is')){
                //Change final 'is' to 'es'
                var s = str.substring(0, str.length - 2);
                str = s + 'es';
                return [qty,str];
            }
            //Ends with 'us'
            else if (str.endsWith('us')){
                //look for exceptions first oExceptions
                for (var i = 0; i < usExceptions.length; i++) {
                    if(str === usExceptions[i][0]){
                        return [qty,usExceptions[i][1]];
                    }
                }
                //Change final 'us' to 'i'
                var s = str.substring(0, str.length - 2);
                str = s + 'i';
                return [qty,str];
            }
            //Ends with 'um'
            else if (str.endsWith('um')){
                //look for exceptions first oExceptions
                for (var i = 0; i < umExceptions.length; i++) {
                    if(str === umExceptions[i][0]){
                        return [qty,umExceptions[i][1]];
                    }
                }
                //Change final 'um' to 'a'
                var s = str.substring(0, str.length - 2);
                str = s + 'a';
                return [qty,str];
            }  
            //Ends with 'a' but not 'ia'  
            else if (str.endsWith('a')){
                //not ending is 'ia'
                if (str.endsWith('ia')){
                    str = str + 's';
                    return [qty,str];
                }
                //look for exceptions first aExceptions
                for (var i = 0; i < aExceptions.length; i++) {
                    if(str === aExceptions[i][0]){
                        return [qty,aExceptions[i][1]];
                    }
                }
                //Change final 'a' to 'ae'
                var s = str.substring(0, str.length - 2);
                str = s + 'a';
                return [qty,str];
            }                 
            //Ends with 'on'  Change final 'on' to 'a'
            else if (str.endsWith('on')){
                //look for exceptions first onExceptions
                for (var i = 0; i < onExceptions.length; i++) {
                    if(str === onExceptions[i][0]){
                        return [qty,onExceptions[i][1]];
                    }
                }
                //Change final 'um' to 'a'
                var s = str.substring(0, str.length - 2);
                str = s + 'a';
                return [qty,str];
            }
            //Ends with 'ex'
            else if (str.endsWith('ex')){
                //look for exceptions first onExceptions
                for (var i = 0; i < exExceptions.length; i++) {
                    if(str === exExceptions[i][0]){
                        return [qty,exExceptions[i][1]];
                    }
                }
                //Change final 'ex' to 'ices'
                var s = str.substring(0, str.length - 2);
                str = s + 'ices';
                return [qty,str];
            }
            else {
                //check unchanging
                for (var i = 0; i < unchanging.length; i++) {
                    if(str === unchanging[i]){
                        return [qty,str];
                    }
                }                    
                //check onlyPlurals
                for (var i = 0; i < onlyPlurals.length; i++) {
                    if(str === onlyPlurals[i]){
                        return [qty,str];
                    }
                }
                //check irregular
                for (var i = 0; i < irregular.length; i++) {
                    if(str === irregular[i][0]){
                        return [qty,irregular[i][1]];
                    }
                }
                str = str + 's';
                return [qty,str];
            }
        } else {
            return [qty,str];
        }
    }

    function isArrayOfArrays(arr){
        if(Array.isArray(arr)){
            var result = true;
            for (var i = 0; i < arr.length; i++) {
                if(!Array.isArray(arr[i])){
                    result = false;
                    //throw expectedArrayOfArrays;
                }
            }
            if(result){
                return true;
            } else {
                //throw expectedArrayOfArrays;
                return false;
            }
        } else {
            return false;
        }
    }
    //API
    return pub;
}());
\$\endgroup\$
2
  • \$\begingroup\$ Why "pluralater" rather than "pluraliser" / "pluralizer"? The latter seems more correct English to me. \$\endgroup\$ – Stuart J Cuthbertson May 25 '16 at 17:33
  • \$\begingroup\$ @StuartJCuthbertson You're right. Thanks for the feedback. I changed it to pluralizer. I like that better. \$\endgroup\$ – Ronnie Royston May 25 '16 at 19:00
1
\$\begingroup\$

Overview

In general, I find the code well organised and easy to read. I especially like that the "private" functions are encapsulated, and hidden from the outside world. Also, the logic of the functions is easy to understand. There is one major change, though I would suggest: namely the format function is too long in my opinion, and could be factored to smaller functions. See below for details.

Decomposing format function

This function basically consists of many if/else-if blocks with the logic:

if (str.endsWith(LETTER1) || str.endsWith(LETTER2) ||  ...){
     // do this
}
else if (str.endsWith(LETTER3) || str.endsWith(LETTER4) ||  ...){
     // do that
}
else if ...

As a first step, I recommend wrapping the // do this and // do that parts into separate (private) functions, and call those functions in the bodies of if/else statements. That should already shorten the function considerably. Note, that at least 3-4 of the exception cases look the same way, i.e.:

//look for exceptions first oExceptions
for (var i = 0; i < umExceptions.length; i++) {
    if(str === umExceptions[i][0]){
         return [qty,umExceptions[i][1]];
    }
}
//Change final 'um' to 'a'
var s = str.substring(0, str.length - 2);
str = s + 'a';
return [qty,str];

These could all be wrapped in a single function, something like:

function handleExceptions(str, qty, exceptionsArr, singularSuffixLen, pluralSuffix) {
    for (var i = 0; i < exceptionsArr.length; i++) {
        if(str === exceptionsArr[i][0]){
             return [qty,exceptionsArr[i][1]];
        }
    }

    var s = str.substring(0, str.length - singularSuffixLen);
    str = s + pluralSuffix;
    return [qty,str];
}

The second step would be to set up a map between singular suffixes and the corresponding functions to process words with those suffixes. This map can be used in format. Something like this:

var pluralizeMap = {
    's'  :  pluralizeEs,
    'x'  :  pluralizeEs,
    'ch' :  pluralizeEs,
    'sh' :  pluralizeEs,
    'z'  :  pluralizeEs,
    'y'  :  pluralizeIes,
    ...
}

function pluralizeEs(str, qty) {
    return handleExceptions(str, qty, xExceptions, 0, "es");
}

function pluralizeIes(str, qty) {
    // ...
}

// ...

In this case the "core" of the function format (i.e., the whole if/else list) would look like this:

for (var suffix in pluralizeMap) {
  if (pluralizeMap.hasOwnProperty(suffix)) {
    var fun = pluralizeMap[suffix];
    return fun(str, qty);
  }
}
pluralizeGeneral(str, qty); // this accounts for the else part

N.B.: I did not try out the full refactoring, so there might be special cases that I overlooked. However, I am convinced that with some tweaking the idea can be applied.

Other remarks

  • Parameters of format: in my opinion, wrapping the parameters in an array makes it slightly more uncomfortable for the user, to invoke this public API function. I guess the reason was that in this way, it is easier to call the format function from read. However, I would recommend making it easier for the user of the API to call the function, even by complicating the internal code a bit (e.g. the calls in read could look like this: format.apply(this, arr[0]), if format has the parameters format(str, qty)).

  • Large if-blocks with small else-parts: in both API methods, there are constructs like if (condition) { /* huge code */ } else { /* one line of code */ }. In my opinion, it would make the code slightly more readable, if it were organised as follows: if (!condition) { /* one line of code */} /* huge code */. In this way, the "huge code" part, would be one level less deep, and thus easier to read. This advice might not apply anymore, if you do the refactoring suggested above.

  • Naming: I think read and format don't do exactly what the name suggests. I would recommend something like pluralizeAllWords and pluralizeOneWord (or something similar).

  • The module returns a help string directly. I would recommend wrapping this in a getHelp() function, so that the clients cannot modify it.

  • Commented out code/unnecessary comments: I recommend removing comments like //throw expectedArrayOfArrays;, API etc., in order to make the code a bit shorter/cleaner.

  • In the function isArrayOfArrays, the loop could be stopped, once you find the first place which is not an array: for (var i = 0; result && (i < arr.length); i++)

Final word: automated testing

In case you have not yet done so, I recommend looking up a JavaScript unit testing framework (e.g. Jasmine, or Karma -- see here for a complete list), and covering at least the following cases: - Generic pluralization ( -s) - One test case for each possible suffix, as per the map above (i.e. at least one test case for single words ending in -s, one for words ending in -x, one for -ch, ...etc). - Edge cases, like empty string, space, non-existing words... of course in such cases there is no "right output". What should be ensured is that the program does not crash, or enter in infinite loop, or throw some nasty exception etc. In other words, even invalid input should be handled "gracefully".

Also, I actually recommend starting with this part (i.e. testing), in case you decide to do the suggested changes. In this way, you will notice immediately if something gets broken in the process.

\$\endgroup\$

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.