Return the longest word (or words) in a string I'm coding for a function that checks the longest word (or words) in a string:


If multiple words are of the same size, those words have to be displayed.
If a single word is inputted, that word has to be displayed.
Words should consist only of alphabets.


The following code has been tested and displays the correct input for the above conditions:

    //this method returns the longest word (or words) of a single string
    static void longestWord(string s)
    {
        int j = 0, k = 0, l = 0, tempLength = 0, maxLength = 0;
        List&lt;int&gt; indexList = new List&lt;int&gt;();
        char[] words = s.ToCharArray();
        StringBuilder longestWord = new StringBuilder();

        //a for loop to ignore spaces at the beginning of the string
        for (int i = 0; i &lt;= s.Length - 1; i++)
        {
            if (((words[i] &gt;= 'A' &amp;&amp; words[i] &lt;= 'Z') || (words[i] &gt;= 'a' &amp;&amp; words[i] &lt;= 'z')))
            {
                j = i;
                l = i;
                break;
            }
        }     

        //a for loop to get the maximum length of the longest word
        for (; j &lt;= s.Length - 1; j++)
        {
            if (((words[j] &gt;= 'A' &amp;&amp; words[j] &lt;= 'Z') || (words[j] &gt;= 'a' &amp;&amp; words[j] &lt;= 'z')))
            {
                tempLength++;

                if (j == (s.Length - 1))
                {
                    if (tempLength &gt;= maxLength)
                    {
                        maxLength = tempLength;  //this ensures the maximum length of a single word is captured
                                                 //removing this will not display single words without non-alphabets
                    }
                }
            }
            else
            {
                if (tempLength &gt;= maxLength)
                {
                    maxLength = tempLength;
                }
                tempLength = 0;
            }
        }

        tempLength = 0;

        //a for loop to add the longest words into a list
        for (; l &lt;= s.Length - 1; l++)
        {
            if (((words[l] &gt;= 'A' &amp;&amp; words[l] &lt;= 'Z') || (words[l] &gt;= 'a' &amp;&amp; words[l] &lt;= 'z')))
            {
                tempLength++;

                if (l == (s.Length - 1))
                {
                    if (tempLength &gt;= maxLength)
                    {
                        indexList.Add(l+1); //'l+1' ensures a word that has no non-alphabet after it is captured
                                            //removing l+1 displays as 'wanted remov' - 'e' is missing
                        k++;
                    }
                }
            }
            else
            {
                if (tempLength == maxLength)
                {
                    indexList.Add(l);
                    k++;
                }
                tempLength = 0;
            }
        }

        //a for loop to append the longest words into a single word
        for (int m = 0; m &lt;= k - 1; m++)
        {
            for (int n = (indexList[m] - maxLength); n &lt; indexList[m]; n++)
            {
                longestWord.Append(words[n]);
            }
            longestWord.Append(" ");
        }

        Console.WriteLine(longestWord);
    }


Honestly, although this code works, I'm unsatisfied with it and would like suggestions as to how I could improve it lengthwise and in readability.
Why do you have more parenthesis than needed in this statement?


if (((words[j] &gt;= 'A' &amp;&amp; words[j] &lt;= 'Z') || (words[j] &gt;= 'a' &amp;&amp; words[j] &lt;= 'z')))



if ((words[j] &gt;= 'A' &amp;&amp; words[j] &lt;= 'Z') || (words[j] &gt;= 'a' &amp;&amp; words[j] &lt;= 'z'))


Similarly:


if (l == (s.Length - 1))



Should be written as:

if (l == s.Length - 1)


As well:


for (int n = (indexList[m] - maxLength); n &lt; indexList[m]; n++)



Should be:

for (int n = indexList[m] - maxLength; n &lt; indexList[m]; n++)




Using j, k and l as a variable in this manner is frowned upon. These should only ever be used with local iterators. (I.e. where you have i.)



Some of your names are not idiomatic.


char[] words = s.ToCharArray();



Should be:

char[] characters = s.ToCharArray();




The string s parameter should be string input (et. al.).



You can take some of your operations to functions, delegates, Lambdas, etc.


if (((words[l] &gt;= 'A' &amp;&amp; words[l] &lt;= 'Z') || (words[l] &gt;= 'a' &amp;&amp; words[l] &lt;= 'z')))



Can more easily be written as a delegate, function, etc.

if (IsAlphaChar(words[l])




Use a foreach loop in place of:


for (int m = 0; m &lt;= k - 1; m++)



Should be:

foreach (int index in indexList)


Then just use index in place of indexList[m].



Never do this:


int j = 0, k = 0, l = 0, tempLength = 0, maxLength = 0;



That is a readability nightmare. Break each variable to it's own line. (And don't declare all of them in one spot. Declare them when you need them.)



You shouldn't declare iterators outside a for loop. (I'm looking at you, j and l.)



Doing words[i] four times is slower than char currentCharacter = words[i];. (Of course, the location that is used should be a method instead.)



You don't use j or l after their respective loops, so don't define them as the first thing in the method. Instead, define them as:

for (int j = i; j &lt;= s.Length; j++)
for (int l = i; l &lt;= s.Length; l++)