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 indexList = new List();
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 = 'A' && words[i] = 'a' && words[i] = 'A' && words[j] = 'a' && words[j] = 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 >= maxLength)
{
maxLength = tempLength;
}
tempLength = 0;
}
}

tempLength = 0;

//a for loop to add the longest words into a list
for (; l = 'A' && words[l] = 'a' && words[l] = 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++;

Why do you have more parenthesis than needed in this statement?

if (((words[j] >= 'A' && words[j] = 'a' && words[j] <= 'z')))

if ((words[j] >= 'A' && words[j] = 'a' && words[j] <= '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 < indexList[m]; n++)

Should be:

for (int n = indexList[m] - maxLength; n < 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] >= 'A' && words[l] = 'a' && words[l] <= '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 <= 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 <= s.Length; j++)
for (int l = i; l <= s.Length; l++)

StackExchange Code Review Q#104953, answer score: 10