C++ getting length of char array using a second function

Question

I'm trying to get the length of a character array in a second function. I've looked at a few questions on here (1 2) but they don't answer my particular question (although I'm sure something does, I just can't find it). My code is below, but I get the error "invalid conversion from 'char' to 'const char*'". I don't know how to convert my array to what is needed.

#include <cstring>
#include <iostream>

int ValidInput(char, char);

int main() {
    char user_input; // user input character
    char character_array[26];
    int valid_guess;
    valid_guess = ValidGuess(user_input, character_array);
    // another function to do stuff with valid_guess output
    return 0;    
}

int ValidGuess (char user_guess, char previous_guesses) {
    for (int index = 0; index < strlen(previous_guesses); index++) {
        if (user_guess == previous_guesses[index]) {
            return 0; // invalid guess
        }
    }
    return 1; // valid guess, reaches this if for loop is complete
}

Based on what I've done so far, I feel like I'm going to have a problem with previous_guesses[index] as well.

Answer 1

char user_input;

defines a single character

char character_array[26];

defines an array of 26 characters.

valid_guess = ValidGuess(user_input, character_array);

calls the function

int ValidGuess (char user_guess, char previous_guesses)

where char user_guess accepts a single character, lining up correctly with the user_input argument, and char previous_guesses accepts a single character, not the 26 characters of character_array. previous_guesses needs a different type to accommodate character_array. This be the cause of the reported error.

Where this gets tricky is character_array will decay to a pointer, so

int ValidGuess (char user_guess, char previous_guesses)

could be changed to

int ValidGuess (char user_guess, char * previous_guesses)

or

int ValidGuess (char user_guess, char previous_guesses[])

both ultimately mean the same thing.

Now for where things get REALLY tricky. When an array decays to a pointer it loses how big it is. The asker has gotten around this problem, kudos, with strlen which computes the length, but this needs a bit of extra help. strlen zips through an array, counting until it finds a null terminator, and there are no signs of character_array being null terminated. This is bad. Without knowing where to stop strlen will probably keep going¹. A quick solution to this is go back up to the definition of character_array and change it to

char character_array[26] = {};

to force all of the slots in the array to 0, which just happens to be the null character.

That gets the program back on its feet, but it could be better. Every call to strlen may recount (compilers are smart and could compute once per loop and store the value if it can prove the contents won't change) the characters in the string, but this is still at least one scan through every entry in character_array to see if it's null when what you really want to do is scan for user_input. Basically the program looks at every item in the array twice.

Instead, look for both the null terminator and user_input in the same loop.

int index = 0;
while (previous_guesses[index] != '\0' ) {
    if (user_guess == previous_guesses[index]) {
        return 0; // prefer returning false here. The intent is clearer
    }
    index++;
}

You can also wow your friends by using pointers and eliminating the need for the index variable.

while (*previous_guesses != '\0' ) {
    if (user_guess == *previous_guesses) {
        return false; 
    }
    previous_guesses++;
}

The compiler knows and uses this trick too, so use the one that's easier for you to understand.

For 26 entries it probably doesn't matter, but if you really want to get fancy, or have a lot more than 26 possibilities, use a std::set or a std::unordered_set. They allow only one of an item and have much faster look-up than scanning a list one by one, so long as the list is large enough to get over the added complexity of a set and take advantage of its smarter logic. ValidGuess is replaced with something like

if (used.find(user_input) != used.end())

Side note: Don't forget to make the user read a value into user_input before the program uses it. I've also left out how to store the previous inputs because the question does as well.

¹ I say probably because the Standard doesn't say what to do. This is called Undefined Behaviour. C++ is littered with the stuff. Undefined Behaviour can do anything -- work, not work, visibly not work, look like it works until it doesn't, melt your computer, anything -- but what it usually does is the easiest and fastest thing. In this case that's just keep going until the program crashes or finds a null.