Ex7 b #13
Ex7 b #13
Conversation
ex7/ex7.js
Outdated
| search = { charValue: '', charCount: 0}, | ||
| doNotSearchList = []; | ||
|
|
||
| if (str != '') { |
There was a problem hiding this comment.
I like to do this instead, because then the edge-case is being considered at the top, while the rest of the code remain simple (i.e. without the indentation):
if (str === '') {
return null;
}
ex7/ex7.js
Outdated
| if (str != '') { | ||
|
|
||
| // Loop through all chars in str | ||
| for (var i = 0; i < str.length; i++) { |
There was a problem hiding this comment.
jasmine version do not have let
ex7/ex7.js
Outdated
| doNotSearchList.push(search.charValue); | ||
|
|
||
| index = 1; | ||
| index = str.indexOf(search.charValue, i + index); |
There was a problem hiding this comment.
Same as in the other question -- this will be O(n*n) = O(n^2) order of n squared. Switch to object-based searching instead.
ex7/ex7.js
Outdated
| doNotSearchList.push(search.charValue); | ||
|
|
||
| index = 1; | ||
| index = str.indexOf(search.charValue, i + index); |
|
|
||
| objCharCount = {}; | ||
|
|
||
| if (str === '') { |
There was a problem hiding this comment.
Do this if first, before the objCharCount = {};
|
|
||
| function mostFrequentChar(str) { | ||
|
|
||
| objCharCount = {}; |
There was a problem hiding this comment.
missing var -- this is now defined on the window :(
| for (var i = 0; i < str.length; i++) { | ||
|
|
||
| // The char we are now working on | ||
| currentChar = str.charAt(i); |
|
|
||
| } else if (maxCount === currentCount) { | ||
|
|
||
| frequentChar = null; |
There was a problem hiding this comment.
This is not clear. I would instead add another variable called moreThanOneMax and set it to true here (then check for it below).
Also, I think this will fail on this instance: 1122333.
2 participants
No description provided.