Implementing StrStr()

This is a naive implementation - I know that there are algorithms like KMP

However I was trying to implement it as best as I can.

string test = "giladdarmonwhatareyoudoing";
int index = StrStr(test, "are");

//Returns an index to the first occurrence of str2 in str1,
//or a -1 pointer if str2 is not part of str1.

public int StrStr(string test, string strToFind)
{
    for (int i = 0; i < test.Length; i++)
    {
        if (test[i] == strToFind[0])
        {
            int j;
            for (j = 0; j < strToFind.Length; j++)
            {
                if (test[i + j] != strToFind[j])
                {
                    break;
                }
            }
            if (j == strToFind.Length)
            {
                return i;
            }
        }
    }
    return -1;
}

Algorithm

-
your implementation has some serious bugs and unexpected behaviour.

-
If passing String.Empty as second parameter, it throws an IndexOutOfRangeException

-
If passing the unique ending part of the first parameter appended by at least one character to the second parameter, it throws an IndexOutOfRangeException

string test = "giladdarmonwhatareyoudoing";
int index = StrStr(test, "ng1");

-
If you are passing null as either first or second parameter a NullReferenceException is thrown. Here it would be better to throw an ArgumentNullException by using a guard clause.

if (test == null) { throw new ArgumentNullException("test"); }
if (strToFind == null) { throw new ArgumentNullException("strToFind"); }

-
your implementation could be improved by

• checking if strToFind.Length > test.Length

• checking if test.Length - i > strToFind.Length

Naming

You shouldn't use hungarian notation. Consider to rename strToFind to searchForm or searchArgument.

Refactoring

After extracting the inner loop to a separate method, removing the now unneeded if (test[i] == strToFind[0]) and implementing the above we will get

public int StrStr(string value, string searchArgument)
{
    if (value == null) { throw new ArgumentNullException("value"); }
    if (searchArgument == null) { throw new ArgumentNullException("searchArgument"); }
    if (searchArgument.Length == 0) { return 0; }

    int searchLength = searchArgument.Length;
    int length = value.Length;

    if (searchLength > length) { return -1; }

    for (int i = 0; i < length; i++)
    {
        if (length - i < searchLength) { return -1; }

        if (IsMatchAtIndex(value, searchArgument, i)) { return i; }
    }
    return -1;
}

private bool IsMatchAtIndex(String value, String searchArgument, int startIndex)
{
    for (int j = 0; j < searchArgument.Length; j++)
    {
        if (value[startIndex + j] != searchArgument[j])
        {
            return false;
        }
    }
    return true;
}

I prefer the above, but you could also add

if (length - i < searchLength) { return -1; }

inverted as condition to the for loop like

for (int i = 0; i = searchLength; i++)
{
    if (IsMatchAtIndex(value, searchArgument, i)) { return i; }
}

StackExchange Code Review Q#73754, answer score: 11