Finding longest word in a string

Question

I was working on a problem from Coderbyte. The challenge was to write a function that takes in a string and returns the longest word in the string. If two words are the same size then it asks to return the first one. The input will never be empty.

function longestword(str){
    var replaced = str.replace(/[^A-Za-z\s]/g,"");
    var final = replaced.split(" ").sort(function(a,b){return b.length - a.length})
    return final[0];
    }
longestword("This is a string theres two words that are the same size oh no!")
=>'string'

What do you think about my code?

Answer 1

longestword should be named using an interCaps naming convention. Also, replaced and final aren't exactly helpful variable names. However, if you were to have an intermediate result called words, it would be quite clear what that stands for.

It's good practice to terminate all statements with semicolons, even though JavaScript allows them to be omitted.

function longestWord(str){
    var words = str.replace(/[^A-Za-z\s]/g, "").split(" ");
    var wordsByDescendingLength = words.sort(function(a, b) {
        return b.length - a.length;
    });
    return wordsByDescendingLength[0];
}

Note that hyphenated words will have their hyphens dropped, both when comparing lengths and when returning the result.

Answer 2

Short and sweet and elegant. Unfortunately, there can be some issues with it.

Correctness

If two word are the same size then it asks to return the first one.

The implementation will only guarantee this if the sort implementation is stable. That is, during sort, the relative ordering of equal elements must not change, otherwise the original first longest string might get moved.

It seems modern browsers all use stable sort, but this was not always the case, and the relevant documentation can be hard to find.

Performance

The time complexity of sorting is usually \$O(n \log(n))\$ at best. Using sort makes your implementation elegant but inefficient because you don't actually need to sort, you can do better. You can walk through the elements in pass and record the longest string you find. That way the solution will be \$O(n)\$.

Non-English characters

The current implementation discards non-English characters like éèà, so the result can be incorrect for French, German, Hungarian texts, for example.

Formatting, coding style

These are just nitpicks, but the code would be somewhat easier to read if you follow these recommendations (borrowing from standards in other languages like Java, Python):

• Put a space before opening braces
• Put a space after commas
• Align the closing brace with the start of its statement
• Either use semicolon to end all statements always or never, do it consistently. (Related article)

Like this:

function longestword(str) {
    var replaced = str.replace(/[^A-Za-z\s]/g, "");
    var final = replaced.split(" ").sort(function(a, b) { return b.length - a.length; });
    return final[0];
}

Note: in addition, use better names as @200_success explained in his answer.

Answer 3

Just wanted to point out that @200_success's review is excellent, but personally I would go for reduce:

    function longestWord2(str){
        var words = str.replace(/[^A-Za-z\s]/g, "").split(" ");
        var longestWord = words.reduce( function( longestSoFar, currentWord ) {
          return currentWord.length > longestSoFar.length ? currentWord : longestSoFar;
        },"");
        return longestWord;
    }
    
    document.write( longestWord2( 'Code Review welcomes you' ) );

Using reduce will access each word only once. It will also return the first word of the largest length if there are multiple words with the same largest length;

Answer 4

In many cases, the simple solutions are... simpler.

In this case, your solution of splitting the data on space , and locating the longest word, is accurate, but a simpler solution was probably intended. As an up-side, the simpler solution is actually better in terms of time-complexity (it is \$O(n)\$, which means the performance takes twice as long if the string is twice as long. Other solutions take more than twice as long if the input data is twice the size).

Bug: You have a bug in your string clean-up too, because you strip all punctuation, and replace it with the empty string "", you are vulnerable to input like This.is.a.long.string, and you will identify that as one word: Thisisalongstring. If you set the replacement as a space " ", you will be OK.

Note, this solution scans the data only once. Other solutions require that the data is scanned multiple times (once to split, once to sort).

function longestWord(str){
    str = str.replace(/[^A-Za-z\s]/g, " ");
    var maxoffset = 0, maxlen=0, wordoffset = 0;
    for (var i = 0; i < str.length; i++) {
        if (str[i] == " ") {
            wordoffset = i + 1;
        }
        if (i - wordoffset + 1> maxlen) {
            maxoffset = wordoffset;
            maxlen = i - wordoffset + 1;
        }
    }
    return str.substring(maxoffset, maxoffset + maxlen);
}

function textUpdated(source) {
    document.getElementById("outword").value = longestWord(source.value);
}

Enter text: <input type="Text" name="Input" id="data" onInput="textUpdated(this);"/>
<hr>
Longest Word: <input type="Text" id="outword" name="OutWord" value="none" />

Answer 5

As was pointed out by janos, non-Latin characters will be removed by your replace-call. Even English words with hyphens or apostrophes will be mangled. This may be completely intentional; depends on what you consider "a word" and what kind of input you expect.

Still, might be simpler to only split the string. For instance:

function longestWord(string) {
  return string.split(/[\s\d.,!?"_\/`]+/).reduce(function (longestSoFar, currentWord) {
    return currentWord.length > longestSoFar.length ? currentWord : longestSoFar;
  });
}

document.write(longestWord("The apostrophe's a tricky thing."));
document.write("<br>");
document.write(longestWord("But so is a naïve regex!"));

This is of course using konijn's reduce solution.

The above completely avoids replace letting split remove things we don't want. The regex will split on whitespace, digits, and different kinds of punctuation, which keeps words like "coöperation" and hyphenations intact.

But it's certainly not without fault; regexes are tricky. There are certainly things that the code above doesn't handle well. Again, it all comes down to what you consider to be a "word" in your context, and what sort of input you expect.

As a sidenote: Your current replace-regex could be written as /[a-z\s]/i - the i modifier flag mean "case-insensitive. No major difference, just an FYI.