K&R exercise 2-5

Question

"Write the function any(s1,s2), which returns the first location in the string s1 where any character from the string s2 occurs, or -1 if s1 contains no characters from s2. (The standard library function strpbrk does the same job but returns a pointer to the location."

I'm wondering if it would be a bad habit to put a return statement within a loop instead of using "char_match = YES" as I'm doing here:

#define YES 1
#define NO 0
int char_seek(char string[], char string2[])
{
    int i, j;
    int char_match = NO;

    for (i = j = 0; string[i] != '\0' && char_match == NO; ++i){
        while (string2[j] != '\0' && string[i] != string2[j])
            ++j;
        if (string2[j] == '\0')
            j = 0;
        else if (string[i] == string2[j])
            char_match = YES;
    }
    if (char_match == NO)
        return -1;
    else
        return i-1;
}

What about return i-1? Is that bad? Should I find a different way to do this?

Answer 1

To answer your questions:

1. Having multiple return statements is fine.
2. As a potential return statement: return i-1; is fine.

So you could certainly rewrite that code as:

int char_seek(char string[], char string2[])
{
    int i, j;

    for (i = j = 0; string[i] != '\0'; ++i){
        while (string2[j] != '\0' && string[i] != string2[j])
            ++j;
        if (string2[j] == '\0')
            j = 0;
        else if (string[i] == string2[j])
            return i;
    }

    return -1;
}

Or, as I would probably write something like this:

int char_seek(const char *string, const char *string2) {
    for (int i = 0; string[i] != '\0'; ++i)
        for (int j = 0; string2[j] != '\0'; ++j)
            if (string[i] == string[j])
                return i;
    return -1;
}

Which I think is far more readable.

As a general rule, when you're implementing a search function, I think that having a return statement inside the loop should be the preferred notation.

Also, while I know that this is a learning exercise, it's worth noting that there is a very similar function in string.h named strpbrk() that would do almost all of the work for you.