Debugging loop and function - javascript

Shouldn't this loop

for(i=0;i<5;i++){
        for(j=i+1;j<5;j++){

be something in these lines

for(var i=0; i< word.length ;i++){
        for(j=i+1; j<word.length/2; j++){

Can't see what your issue is but here's the solution I suggest for your problem:

var words = ['hello', 'bye', 'foo', 'baz'];

function hasUniqLetters(word){
  var uniq = word.split('').filter(function(letter, idx){
    return word.indexOf(letter) == idx;
  });
  return uniq.length == word.length;
}

var result = words.filter(hasUniqLetters);

console.log(result); //=> ["bye","baz"]

function checkLetters(word){
    for(i=0;i<5;i++){      //both i and j were not instantiated
        for(j=i+1;j<5;j++){
            if(word.charAt(i) == word.charAt(j)){ //It could be crashing if
                return false;                     //word <= 5
                break;//a break after a return
            }         //statement is redundant.
        }
    }
    return true;
}

You must put var before declaring a variable.

Try this:

function checkLetters(word){
  for(var i=0,j=1,l=word.length-1; i<l; i++,j++){
    if(word.charAt(i) == word.charAt(j)){
      return false;
    } 
  }
  return true;
}
var compLibrary = [];
for(var i=0,l=library; i<l; i++){
  if(checkLetters(library[i]) == true){
    compLibrary.push(library[i]);
  }
}

tldr; The code originally posted should not crash the browser.

The following explains why nested loops are not always bad for efficiency and shows a counter-example where the original code works successfully without crashing the browser when running over 100,000 simulated words.

The complexity of the posted code is low and it should run really fast. It executes here in a fraction of a second (under 20ms!), even at all "20 * 8000" - i.e. C * O(n). Note that the time complexity is linear because the nested loops in checkLetters have a constant time: due to this small fixed limit ("20 loops" max each call) it does not represent a performance problem here.

As such, I maintain that it is not an issue wrt it being an efficiency problem. I assert that the original code will not "crash" a modern browser environment. For longer or unbound words then using a (presumably) lower complexity probe attempt may pay off - but the inner loop runs in small constant time here. (Actually, due to distribution of letters within words and word lengths I would imagine that the constant rarely exceeds "90 loops" for a natural language like English.)

See http://jsfiddle.net/FqdX7/6/

library = []
for (w = 0; w < 100000; w++) {
    library.push((w + "12345").slice(0,5))
}
function checkLetters(word){
    for(i=0;i<5;i++){
        for(j=i+1;j<5;j++){
            if(word.charAt(i) == word.charAt(j)){
                return false;
            } 
        }
    }
    return true;
}
$('#time').text("running")
start = +(new Date)
var compLibrary = [];
for (var k=0;k<library.length;k++) {
    if(checkLetters(library[k]) == true) {
        compLibrary.push(library[k]);
    }
}
time = +(new Date) - start
$('#time').text(time + "ms")

On my machine (in Safari) the code runs in ~30 milliseconds (~40ms if the "return false" is removed) for an input of 100,000 words!

In comparison, the answer with a probe (seenChars lookup) actually runs worse in Safari/Chrome. See http://jsfiddle.net/Hw2wr/5/, where for 100k words it takes about 270ms - or about 9x slower. However, this is highly browser dependent and the jsperf in the comments shows that in Firefox the probing approach is faster (by about 2x) but is slower again in IE (say 4-5x).

YMMV. I think the original code is acceptable for the given situation and the "crashing" problem lies elsewhere.