First, your loops are evil to read. It took me several minutes to figure out what the condition did. Essentially, you loop N times, where N is the Nth instance of the character in the string you wish to find the index for. Each time you loop, you find the index of the next occurrence of the character in the string, update the position to search from, and continue the process until the index is either -1 (no Nth instance), or until n > 0
or n < 0
, based on which side you are searching from.
A simpler way to write this algorithm is as follows:
public static int NthIndexOfC(this string input, char charToFind, int n)
{
int position;
switch (Math.Sign(n))
{
case 1:
position = -1;
do
{
position = input.IndexOf(charToFind, position + 1);
--n;
} while (position != -1 && n > 0);
break;
case -1:
position = input.Length;
do
{
position = input.LastIndexOf(charToFind, position - 1);
++n;
} while (position != -1 && n < 0);
break;
default:
throw new ArgumentOutOfRangeException(message: "param cannot be equal to 0", paramName: nameof(n));
}
return position;
}
It takes a little bit more room since I wrote the loop body on its own lines, but it is easy to understand, and there are no assignments in the loop condition.
However, this is still not the way I would write this algorithm. When you think about how IndexOf
works, it iterates the string with a for
loop from the specified starting index until it reaches the first instance of the requested character, which it returns. We can write similar behavior, but just keep track of which index we are at:
public static int NthIndexOf(this string input, char charToFind, int n)
{
int count = 0;
switch (Math.Sign(n))
{
case 1:
for (var index = 0; index < input.Length; index++)
{
if (input[index] == charToFind)
{
count++;
}
if (count == n)
{
return index;
}
}
break;
case -1:
for (var index = input.Length - 1; index >= 0; index--)
{
if (input[index] == charToFind)
{
count++;
}
if (count == Math.Abs(n))
{
return index;
}
}
break;
default:
throw new ArgumentOutOfRangeException(message: "param cannot be equal to 0", paramName: nameof(n));
}
return -1;
}
This is the longest solution yet, but we aren't finished. Notice all that duplicated code?
public static int NthIndexOf(this string input, char charToFind, int n)
{
switch (Math.Sign(n))
{
case 1:
return NthIndexOf(input, charToFind, n, 0, input.Length);
case -1:
return NthIndexOf(input, charToFind, n, input.Length - 1, -1);
default:
throw new ArgumentOutOfRangeException(message: "param cannot be equal to 0", paramName: nameof(n));
}
}
public static int NthIndexOf(string input, char charToFind, int n, int searchFrom, int searchTo)
{
int count = 0;
var index = searchFrom;
while (index != searchTo)
{
if (input[index] == charToFind)
{
count++;
}
if (count == Math.Abs(n))
{
return index;
}
index += Math.Sign(n);
}
return -1;
}
There. Clean, easy to understand and maintain, and lots of options to use. Notice that the second NthIndexOf
needs to have the searchTo
one integer larger or smaller than the value you want to stop at to accommodate the multi-directional search.