0

Alright, so basically this code passes in a sentence into the function and the function needs to figure out which word is longest to return. Everything works great except that the very last letter keeps getting cut off. So what would be a good solution to this problem?

    function LongestWord(sen) { 
sen = sen.toLowerCase();
  var build = "";
  var arr = [];
  var longest = 0;
for(var i = 0; i < sen.length;i++){
  var cur = sen.charCodeAt(i);
  console.log(sen.charCodeAt(i))
  if(i == sen.length - 1){

       arr.push(build);
    }
    if(sen.charAt(i) === " "){
    arr.push(build);
     build = "";
    }
    if(cur >= 97 && cur <= 122){
    build += String.fromCharCode(cur);
    }
  }
  console.log(arr);
  for(var e = 0; e < arr.length - 1;e++){
    if(arr[e].length > arr[e + 1].length){

   longest = arr[e];
    }
    else{
    longest = arr[e + 1];
    }
  }
  return longest; 

}

// keep this function call here 
// to see how to enter arguments in JavaScript scroll down
console.log(LongestWord("Johnny ErsoL"));

It returns "Johnny", which is correct, but this is what the Array looks like at the end.

[ 'johnny', 'erso' ]
SuperCoolDude1337
  • 87
  • 1
  • 9
  • 2
    What is all this? Are you just trying to return an array of all the words in a sentence sorted by number of characters ? – adeneo Aug 06 '14 at 19:13
  • The reason is that you're not reading enough characters. You should be reading the length +1. – durbnpoisn Aug 06 '14 at 19:14

4 Answers4

3

Here's my suggestion ?

function LongestWord(sen) {
    return sen.split(/\b/).filter(function(item) {
        return item.trim().length;
    }).sort(function(a,b) {
        return b.length - a.length;
    });
}

split the sentence on word boundary, then trim off empty spaces, finally sort by length of each word and return the sorted array.

adeneo
  • 312,895
  • 29
  • 395
  • 388
  • I would mark that as the Answer(I'm sure it's correct) but I'm still learning JavaScript as of a few days ago and I'm not sure how that code works (yet!). I don't want to be c+p code, that way i would not learn! – SuperCoolDude1337 Aug 06 '14 at 19:25
  • If you split it up, and read the documentation for `split`, `Array.filter` and `Array.sort` it really is quite simple, a lot more so than your code. – adeneo Aug 06 '14 at 19:35
  • Thank you, i will do some research on those! – SuperCoolDude1337 Aug 06 '14 at 19:42
  • @adeneo Do you have any idea why I would get downvoted? I'm trying to figure it out and can't see any reasonable reason, other than perhaps my solution counts punctuations as words, but yours does too... – plalx Aug 06 '14 at 21:10
  • @plalx - I have no idea why it was downvoted, the answer looks fine to me, I didn't really test it, but I read it carefully, and it seems just fine. It doesn't filter out everything, neither does mine, and it doesn't return an array, but I don't see why that would be that important anyway. – adeneo Aug 06 '14 at 21:54
1

Try replacing 

for(var i = 0; i < sen.length;i++){
  var cur = sen.charCodeAt(i);
  console.log(sen.charCodeAt(i))
  if(i == sen.length - 1){

       arr.push(build);
    }
    if(sen.charAt(i) === " "){
    arr.push(build);
     build = "";
    }
    if(cur >= 97 && cur <= 122){
    build += String.fromCharCode(cur);
    }
  }

with

arr=sen.split();
Larkeith
  • 434
  • 2
  • 9
  • Wow that was such an easy fix, makes me feel stupid for completely forgetting about the split method. Thanks though, I'll mark that as the answer as soon as I'm able to. – SuperCoolDude1337 Aug 06 '14 at 19:23
  • One problem is that the strings can't include any characters besides a - z or numbers. So i used sen.split(" ") to put each word into the array, then how would i go on to check if their are any bad chars in there? – SuperCoolDude1337 Aug 06 '14 at 20:03
  • @user3890432 I found [this question](http://stackoverflow.com/questions/9364400/remove-not-alphanumeric-characters-from-string-having-trouble-with-the-char), which gives `sen.replace(/\W/g, '');` as a simple way to strip all non-alphanumeric chars (except _) from a string. – Larkeith Aug 06 '14 at 20:14
  • I just tried adding it and it did not change the code at all @Larkeith – SuperCoolDude1337 Aug 06 '14 at 20:18
  • @user3890432 Sorry, I forgot to mention that `String.replace()` only returns a new string, it doesn't alter the original - to strip the characters you'll have to use `sen=sen.replace(/\W/g, '');`, and to check if there are any you would use `if (sen==sen.replace(/\W/g, '')) {Do stuff};` – Larkeith Aug 06 '14 at 20:23
  • Oh ok perfect, i created a new var that is equal to sen.replace() and then made sen equal that new var. Only problem is that it seems to have completely removed the space in between the word. Which means the entire String goes into the first element in the array. – SuperCoolDude1337 Aug 06 '14 at 20:28
  • @user3890432 More shameless stealing from [another answer](http://stackoverflow.com/questions/14640486/remove-all-characters-except-alphanumeric-and-spaces-with-javascript): `sen.replace(/[^\w\s]/gi, '');` – Larkeith Aug 06 '14 at 20:36
  • Looks like code is now working, Thanks Larkeith & Stack Overflow! – SuperCoolDude1337 Aug 06 '14 at 20:38
0

You have a for loop with e <arr.length -1. I don't think you need the -1.

Tim
  • 81
  • 8
0

I know you already found your answer, however I just wanted to show you an alternative to what you are doing.

To simplify your code and also make it more understandable, but also flexible, it's usually a good idea to make sure that every function does only one thing, or has a single responsability.

In your code, your LongestWord function has the responsability to identify what is a word and to find out which one is the longest.

What you could have done is create a function that knows how to tokenize a sentence into words:

function forEachWordsIn(str, callback) {
    var rx = /\b(\w+?)\b/g,
        match;

    while (match = rx.exec(str)) callback(match[0]);
}

Then use that words iterator function from the longestWord function, which makes this algorithm extremely trivial now:

function longestWord(sen) {
    var longestWord = '';

    forEachWordsIn(sen, function (word) {
        if (word.length > longestWord.length) longestWord = word;
    });

    return longestWord;
}

Note: I renamed LongestWord to longestWord because making functions start with captital letters is a well known standard to identify constructor functions.

plalx
  • 42,889
  • 6
  • 74
  • 90
  • Oh i see, splitting up the task between functions. One extra thing i forgot to mention is that the words can't have any bad Characters in them (bad Characters = !,@,#,$,%,^,&,*,etc.) Basically it can only have letters in it before it compares the strings and returns the longest one. Any idea on how to do this? example: (arr[i].charCodeAt(e) <= 97){arr[i].splice(e,1)} But that returns an error! – SuperCoolDude1337 Aug 06 '14 at 20:13
  • @user3890432, Yes you can simply change `\S` to `\w` in the iterator regex. This will ensure that only word characters are captured as words. I updated my answer. You can also see how seperating responsability helped with that little change since we do not have to touch the `longestWord` logic. – plalx Aug 06 '14 at 21:14