1

I don't think this is a matter of global-local variables as the duplicate bot suggested because when I edited the wrong code and placed the counter variable inside the for loop, the same wrong output was given.

I was trying to write a function which counts the number of instances an input word/character occurs.

the test data were :

1- input (find('The quick brown fox', 'fox')) , output (“fox was found 1 times”)

2- input (find('aa, bb, cc, dd, aa', 'aa')) , output(“aa was found 2 times”)

I have attempted to solve it two times, first time the output was ”fox was found 6.333333 times ” and the second solution gave an output “fox was found 1 times”.

The difference between the two methods I attempted was the position of the final output counter and the syntax used.

first attempt:

function find(str,key){ 
var count = 0;
var answer = 0;
    for (var i = 0; i<str.length; i++){ 
        if (str[i] == key[0] ){
            for (var j = i; j<i +str.length; j++){
                if (str[j] == key[j-i]){ 
                    count = count + 1;
                }       
            }
        }
    }

    answer = (count)/(key.length);
    return key + "was found " + answer + "times";

}

second attempt:

function find(str,key){ 
var count = 0;
var answer = 0;
    for (var i = 0; i<str.length; i++){ 

        if (str[i] == key[0] ){ 

            for (var j = i; j<i +str.length; j++){    

                if (str[j] == key[j-i]){  
                    count = count + 1;
                }

                if (count == key.length){answer = answer + 1;}
            }
        }
    }
    return key + "was found " + answer + "times";
}

I am not sure why the first one did not work, and what happened inside my code that yielded an answer of 6.3333 rather than 1?

Mohamed Hegazy
  • 285
  • 3
  • 18
  • 1
    [***This***](http://stackoverflow.com/questions/840781/easiest-way-to-find-duplicate-values-in-a-javascript-array) may help you, but you have two different inputs with & without comma. – The Reason Mar 16 '16 at 21:15
  • 1
    You're dividing count by key.length in the first example, hence why you get a decimal value (19/3 = 6.333). The second example only increments answer, hence why it's an integer value. – ManoDestra Mar 16 '16 at 21:17
  • @ManoDestra yes I understand the output can be decimal when division happens, but in my case and if my maths is correct, count should be 3 and key.length also 3, therefore output is supposed to be 1 not 6.3333 – Mohamed Hegazy Mar 16 '16 at 21:19
  • Somehow, it is picking up the length of the input string, rather than the length of the array. – ManoDestra Mar 16 '16 at 21:20
  • @ManoDestra it's obvious that count was calculated to be 19, therefore when divided by 3 the output was 6.3333, so my question was why count was 19 rather than 3 – Mohamed Hegazy Mar 16 '16 at 21:21
  • @ManoDestra yeah, not sure why it counts the string length, probably something wrong with my function – Mohamed Hegazy Mar 16 '16 at 21:22
  • My guess is that you accidentally had a single = in there somewhere instead of ==. That way your variable could have been overwritten with a string variable. And the length would then be 19, rather than the length of the previously stored array. – ManoDestra Mar 16 '16 at 21:26

3 Answers3

1

You have two problems in you first attempt, see comments in the (fixed) code below

function find(str,key){ 
    var count = 0;
    var answer = 0;
    for (var i = 0; i<str.length; i++){ 
        if (str[i] == key[0] ){
            for (var j = i; j<str.length; j++){  // you have to stop at end of "str"!
                if (str[j] == key[j-i]){ 
                    count = count + 1;
                }
                else {
                    break; // no match, you have to restart at beginning of "key"
                }       
            }
        }
    }

    answer = (count)/(key.length);
    return key + "was found " + answer + "times";

}

NB there's a third problem that's potentially fixed by your 2nd version (potentially because there's bugs in it too). The problem is that partial matches should count for zero and not n/key.length (an example is provided in the comments by @TheGreatContini).

While we're at it, here is a fix to the 2nd one

function find(str,key){ 
    var count = 0;
    var answer = 0;
    for (var i = 0; i<str.length; i++){ 

        if (str[i] == key[0] ){ 

            for (var j = i; j<str.length; j++){ // same problem here

                if (str[j] == key[j-i]){  
                    count = count + 1;
                }
                else { 
                    count = 0;
                    break; // you have to restart
                }

                if (count == key.length){
                     count = 0;   // a new beginning
                     answer = answer + 1;
                 }
            }
        }
    }
    return key + "was found " + answer + "times";
}

PS as a bonus, here is a more efficient string matching algorithm - O(str.length) instead of O(str.length*key.length).

Ilya
  • 5,377
  • 2
  • 18
  • 33
  • You could drop the else by moving the if condition into the condition on the for loop. – Trisped Mar 16 '16 at 21:22
  • @Trisped that's right, it would be shorter, but harder to comment the change and point the problem of the original code. – Ilya Mar 16 '16 at 21:27
  • @TheGreatContini the result is `count / key.length`, so it is fine. – Trisped Mar 16 '16 at 21:32
  • Referring to first code only: It is wrong to implement `count` for partial word matches. This code will produce wrong answers for a number of cases. Try for example find('aa aa aa aa', 'ab')). The code should output 0 matches, but this code will not get the right answer. – TheGreatContini Mar 16 '16 at 21:37
  • The second code has issues to: you need to init `count` to 0 every time before the `for j` loop is ran. – TheGreatContini Mar 16 '16 at 21:40
  • @TheGreatContini yes, that's fixed. About the first one, that's another call - it indeed isn't right, but it answers the question for the examples given. – Ilya Mar 16 '16 at 21:41
  • @TheGreatContini I've updated the answer, thanks for your comments. – Ilya Mar 16 '16 at 21:52
  • @Ilya Thank you so much – Mohamed Hegazy Mar 17 '16 at 18:59
1

You're dividing count by key.length in the first example, hence why you get a decimal value (19/3 = 6.333, where 19 is the length of the input). The second example only increments answer, hence why it's an integer value.

ManoDestra
  • 6,325
  • 6
  • 26
  • 50
1

Every time you do a for j loop, you need to initialise count to 0.

A new loop is the beginning of a new word comparison. You only increment answer if count reaches the full length of the word. So you are always going to need a if (count == key.length){answer = answer + 1;}.

BTW, your for i loop should not go up to str.length, instead subtract key.length from it.

TheGreatContini
  • 6,429
  • 2
  • 27
  • 37