1
\$\begingroup\$

I created a function that compares an array of letters to the alphabet, and returns the letters that are not in the original array.

While this works, it looks somewhat clumsy to me, but I don't know what I'm looking for to improve the code...Is there a way to make it more elegant? Perhaps getting rid of the notPresent array somehow?

getLettersNotInContent(['z', 'a', 'p']);

function getLettersNotInContent(letters) {
  // Filter dups if present
  var uniques = [];
  letters.filter(function(letter) {
    if (uniques.indexOf(letter) === -1) {
      uniques.push(letter);
    }
  });

  // Filter and return the letters that are not present in the content
  var alphabet = ['a','b','c','d','e','f','g','h','i','j','k','l','m','n','o',
  'p','q','r','s','t','u','v','w','x','y','z'];
  var notPresent = [];
  alphabet.filter(function(letter) {
    if (uniques.indexOf(letter) === -1) notPresent.push(letter);
  });
  return notPresent;
}
\$\endgroup\$

4 Answers 4

3
\$\begingroup\$

You're mixing up two different ways of doing this. The old-fashioned way involves looping through the alphabet and pushing the letters not present into an array:

function getLettersNotInContent2(letters) {
    var alphabet = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
                    'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'];
    var notPresent = [];
    for (var i = 0; i < alphabet.length; i++) {
        if (letters.indexOf(alphabet[i]) === -1) notPresent.push(alphabet[i]);
    }
    return notPresent;
}

The newer way (more elegant but not supported in IE8) uses the filter method with a function that returns true or false:

function getLettersNotInContent3(letters) {
    var alphabet = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm',
                    'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'];
    return alphabet.filter(function (letter) {
        return letters.indexOf(letter) === -1
    });
}

Your code happens to work because, under the hood, filter also loops through the values of an array in turn. But you're not using it in the intended way.

(In both cases I removed the filtering out of duplicates as this seems unnecessary.) (jsfiddle)

\$\endgroup\$
6
  • \$\begingroup\$ Using indexOf is actually quite slow. You should probably be building a map first to allow quick lookups. \$\endgroup\$
    – plalx
    Oct 14, 2013 at 4:53
  • \$\begingroup\$ @plalx That would be very premature optimisation. We are talking about up to 26 letters. There are unlikely to be performance problems resulting from this function, and building a map is not the most likely solution if there are. \$\endgroup\$
    – Stuart
    Oct 14, 2013 at 8:41
  • \$\begingroup\$ I would call it writing efficient code rather than premature optimization as it wouldn't make the implementation much more complex and the resulting would be an O(n) algorithm rather than O(n^2). \$\endgroup\$
    – plalx
    Oct 14, 2013 at 13:56
  • \$\begingroup\$ @plalx No it's actually slower, because in this case you have to make a new hash for each value of letters. jsperf.com/filter-with-hash-vs-with-indexof. Of course it may depend on the length of letters and whether the hashes can be stored and reused somehow. \$\endgroup\$
    – Stuart
    Oct 14, 2013 at 18:49
  • \$\begingroup\$ You seem to be right that it isin't true in Chrome, but it really doesn't make any sense. It takes O(n) time to create the map, but then lookups takes O(1) time. It should definitely be faster. I tested in IE10 for example and the hash runs 30% faster. jsperf.com/filter-with-hash-vs-with-indexof/2 \$\endgroup\$
    – plalx
    Oct 14, 2013 at 19:01
2
\$\begingroup\$

Most People don't like regular expressions as they can get extremly complex. but I think this is a perfectly simple example where it could be used.

function getLettersNotInContent(letters)
{
    var alphabet = "abcdefghijklmnopqrstuvwxyz";                 // Line 1
    var regexString = "[" + letters.join("|") +"]";              // Line 2
    var regexUnwantedChars = new RegExp(regexString, "gi");      // Line 3
    var resultString = alphabet.replace(regexUnwantedChars , "");// Line 4
    var resultArray = resultString.split("");                    // Line 5
    return resultArray;                                          // Line 6
}

console.log(getLettersNotInContent(["z", "a", "p"]));
console.log(getLettersNotInContent(["a", "b", "c"]));
console.log(getLettersNotInContent(["x", "y"]));
console.log(getLettersNotInContent("abcdefghijklmnopqrstuvwxyz".split("")));
console.log(getLettersNotInContent("abcdefghijklnopqrstuvwxyz".split("")));

I split the function into many lines to make it easy to explain but It needn't be so verbose.

  1. you need the alphabet in a string. (line 1)
  2. create a RegExp object. (lines 2-3)
  3. remove characters. (line 4)
  4. split what's left by character. (line 5)

compact version:

function getLettersNotInContent(letters)
{
    return "abcdefghijklmnopqrstuvwxyz"
        .replace(new RegExp("[" + letters.join("|") +"]", "gi"), "")
        .split("");
}
\$\endgroup\$
1
\$\begingroup\$

Shortest way i could think of.

  1. theoretically the while-- loop is faster in the most browsers
  2. this alphabet has 26 letters so why not set it directly
  3. you can jump to a specific letter inside a string in javascript using the index like in arrays
  4. the shorthand inverse operater shortens also a little the check inside the while loop

you can pass ['a','x','u'] or 'axu' as letters value

and by replacing b=[] & b.push(a[l]) with b='' & b+=a[l] you get a string.

then you can split it with b.split().

note: as you use while-- the letters ar not ordered alphabetically .. to do that you need to invert the a string or use b.sort()

function missingLetters(letters,a,b,l) {//a,b,l = placeholder so no need for var
 l=26;
 a='abcdefghijklmnopqrstuvwxyz';
 b=[];
 while(l--){
  letters.indexOf(a[l])!==-1||b.push(a[l])
 }
 return b;
}

Example

http://jsfiddle.net/uhqWh/

\$\endgroup\$
0
\$\begingroup\$

If we split an alphabet string which looks nicer IMO, and use the "not -1 operator", we get this:

function missingLetters( s )
{
  return "abcdefghiklmnopqrstuvwxyz".split("").filter( function(c) 
  {
    return !~s.indexOf(c)
  });
}
\$\endgroup\$

Your Answer

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

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