3
\$\begingroup\$

I am working on trying to create a range function in JavaScript that imitates PERL's 'a'..'zz'.

which expands to:

  1. a->z
  2. aa->az
  3. ba->bz
  4. ...
  5. za->zz

And so on for every extra character you add to the end of the range( e.g zzz .... ).

I have a working implementation, However, I think that there is a more efficient - and perhaps more idiomatic - solution.

There are two things I would like answers to address:

  1. I would like to reduce the code size - this seems like too many lines for the problem that it solves.
  2. I would like improve the performance - the implementation that I have is very slow for any end string > 5 characters long.

function charRange( start, end ) {
  var current = start;
  var range   = [];

  range.push( current ); //Put the start directly into the range.
  if ( typeof start === typeof end &&
     ( typeof start === "string" )) {
    if ( typeof end !== "string" ) {
        console.log( "ERROR: start and end must both be Strings!" );
    }
  }

  while ( !stringEqual(current.split(""), end.split("")) ) {
    current = next(current);
    range.push( current );
    // process.exit();
  }

  return range;


  function stringEqual( chars, testChars ) {
    var isEqual = 1;
    var sum = 0;
    var testSum = 0;
    var i = 0;
    while ( i < testChars.length ) {
        // console.log(chars);
        // console.log(testChars);
        // console.log(i);

        if ( isEqual && chars[i] !== testChars[i] ) {
            isEqual = 0;
        };
        if ( chars[i] ) {
            sum     += chars[i].charCodeAt(0);
        };
        testSum += testChars[i].charCodeAt(0);
        i++;
    }

    // console.log( sum + " " + testSum );
    return isEqual || (sum < testSum ? 0 : 1);
  }

  function allAreZ( string ) {
    var found = 1;
    var i     = 0;
    while ( i < string.length ) {
        if ( string[i] !== "z" ) {
            found = 0;
            break;
        }
        i++;
    }

    return found;
  }

  function findIncrementIndex( item ) {
    var index = item.indexOf("z");
    var i     = 0;

    while (index === 0) {
        i++;
        index = item.substr(i).indexOf("z");
    }

    index += i;

    return index;
  }

  function next( item ) {
    if ( typeof item === "number" ) {
        return item++;
    } else if ( typeof item === "string" ) {
        var lastCharCode = item.charCodeAt( item.length -1 );
        item = item.toLowerCase();

        if ( lastCharCode === "z".charCodeAt( 0 ) ) {
            if ( allAreZ(item) ) {
                // console.log( "Reached the end, adding a letter");
                return Array(item.length +2).join("a");    
            } else { 
                var index = findIncrementIndex(item);
                var charCode = item.charCodeAt( index -1 );
                var newChar = String.fromCharCode( ++charCode );
                item = item.replace(/[a-y]z/, newChar + "a").replace(/z+$/, "a");
                // console.log( "Reached the end, next set is: " + item);
                return item;
            }

        } else {
            var newChar = String.fromCharCode( ++lastCharCode );
            // console.log( "Next Character is: " + newChar + " Code: " + lastCharCode + " Input: " + item);
            return item.replace( /\w$/, newChar );
        }
    }
  }
};
\$\endgroup\$
5
  • 1
    \$\begingroup\$ Related: Iterate from “0” to “ZZZZZ” \$\endgroup\$
    – Flambino
    Sep 22, 2014 at 12:16
  • \$\begingroup\$ This may be helpful... codereview.stackexchange.com/a/44094/38054 converts numbers to letters \$\endgroup\$
    – BenVlodgi
    Sep 22, 2014 at 16:11
  • \$\begingroup\$ Thanks, fellows. Those are both better ideas than mine. I think that I am going to go with Flambino's. Can you make that an answer so I can mark it? \$\endgroup\$
    – esyphelon
    Sep 22, 2014 at 16:19
  • \$\begingroup\$ Hey, I see you got a very nice review from konijn, so I'll just let my comment remain a comment. Glad you found it useful, though :) By the way, if you add a @ in front of a user's name, that user will be notified about your comment (the user whose question/answer you're commenting on will always be notified by default). Useful if you're trying to contact someone (like me, in this case) \$\endgroup\$
    – Flambino
    Sep 22, 2014 at 20:48
  • \$\begingroup\$ Okay, so my updated example has one caveat, it is not in order. Still working on figuring that one out. \$\endgroup\$
    – esyphelon
    Sep 23, 2014 at 17:34

1 Answer 1

5
\$\begingroup\$

From a once over:

  • As a caller I would rather whether start or end was not a string instead of getting ERROR: start and end must both be Strings!
  • Indentation! Please consider using something like jsbeautifier
  • You can merge these

    var range = [];
    range.push(current); //Put the start directly into the range.
    

    into

    var range = [start]; //Begin with the start <- Perhaps too zen a comment
    
  • Do not keep commented out code
  • I cannot understand why you would need

    while(!stringEqual(current.split(""), end.split(""))) {
    

    why can't you simply

    while(current != end) {
    
  • Also, this, I cant see when you would need this:

    if ( typeof item === "number" ) {
        return item++;
    

    Since you verify that both start and end are string, this should never happen?

  • allAreZ is written in a convoluted way, in this type of function ( small, and dedicated to 1 task), it is appropriate to return immediately. You could try something like

    function allZ( string ) {
        var i = 0;
        while ( i < string.length ) {
            if ( string[i] !== "z" ) {
                return false; 
            }
            i++;
        }
        return true;
    }
    

    or, you something fun like this

    function allZ( string ) {
        return string.split('z').length == string.length + 1;
    }
    

I am mulling about writing something far shorter with .toString(26) To be sure zzzzzz equals 308915775, that is a lot of entries to store in array, in fact at this point it is silly to store this into an array. You would be better of writing an iterator class which provides a next() function for ranges in that size.

function charRange(start, end)
{
  if(typeof start !=  "string" ){
    throw 'start must be a string';
  }
  
  if(typeof end !=  "string" ){
    throw 'end must be a string';
  }

  var range = [],
      current,
      mapRadix = {},
      mapAlphabet = {},
      a = 'a'.charCodeAt(0);
  
  for( var i = 0 ; i < 26 ; i++ ){
    mapRadix[i.toString(26)] = String.fromCharCode(a+i);
    mapAlphabet[String.fromCharCode(a+i)] = i.toString(26);
  }
  
  function string2Number( s ){
    var i26 = '';
    for( var i = 0 ; i < s.length ; i++){
      i26 += mapAlphabet[ s[i] ];
    }
    return parseInt(i26,26);
  }
  
  function number2String( int ){
    var n = int.toString(26),
        s = '';
    for( var i = 0 ; i < n.length ; i++){
      s += mapRadix[ n[i] ];
    }
    return s;
  }
  
  current = string2Number( start );
  end = string2Number( end ) + 1;
  if( current > end ){
    throw 'start must be smaller than end';
  }
    
  while(current != end ) {
    range.push(number2String(current++));
  }
               
  return range;
}

\$\endgroup\$
8
  • \$\begingroup\$ Thanks for the help. Sorry about the formatting issues, I was having some issues getting the code into SO's format from my editor. Only reason that I wrote StringEqual is to avoid an infinite loop(start is something that will never increment into end). The typeof === "number" I thought I had removed(had wanted to create a range function initially, but limited it to chars later). \$\endgroup\$
    – esyphelon
    Sep 22, 2014 at 18:50
  • \$\begingroup\$ Updated with an alternative, you can use parts of it to write a true range iterator. \$\endgroup\$
    – konijn
    Sep 22, 2014 at 19:14
  • \$\begingroup\$ +1. I especially like "Begin with the start". Ancient wisdom in that comment :) \$\endgroup\$
    – Flambino
    Sep 22, 2014 at 20:34
  • \$\begingroup\$ So after testing your solution a little more, I found that it goes straight from 'z' -> 'ba', which is not quite the wanted behavior(which would be 'z' -> 'aa'..'az'->'ba'). I have been trying to find a method to circumvent this, but have had no luck. \$\endgroup\$
    – esyphelon
    Sep 24, 2014 at 17:06
  • \$\begingroup\$ Lemme look at that \$\endgroup\$
    – konijn
    Sep 24, 2014 at 17:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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