3
\$\begingroup\$

Is it considered bad practice to pass string literals as char*, when an array and not a c-string is expected by the function? From my code:

char ConsoleIO::GetNextChar(string prompt, char* choices, int numChoices) {
    // Loop until valid input, or 10 times.
    for (int i = 0; i < 10; i++) {
        Write(prompt);

        char* buffer = new char[200];
        cin.getline(buffer, 200);
        char choice = tolower(buffer[0]);

        if (in(choice, choices, numChoices)) {
            return choice;
        }

        WriteLine("Invalid choice.");
    }
    // Just return the first choice as the default.
    return choices[0];
}

// In main:
char userChoice = ConsoleIO::GetNextChar("Make a selection: ", "234", 3);

It compiles, but is it bad practice?

\$\endgroup\$
  • 1
    \$\begingroup\$ How does this even compile? "234" is a const char[] which cannot be converted to a char *. You should at least get a warning. \$\endgroup\$ – nwp Dec 6 '15 at 11:16
  • \$\begingroup\$ Nope, I don't even get a warning on this. My guess is that since it's not by-ref the constness can be ignored, and char[] and char* are really the same thing in c++, it allows me to pass a const char[] as a char*. \$\endgroup\$ – Java Matrix Dec 7 '15 at 15:30
5
\$\begingroup\$

When a C or C-related programmer sees char* they're going to naturally think of passing in a C-string. In my opinion, it's absolutely fine to do that. If you want to invoke an array of characters in C, I'd do it like this:

char ConsoleIO::GetNextChar(string prompt, char[] choices, int numChoices)

But, you're in C++ so you should use a std::vector<char> and then don't pass in numChoices, like this:

char ConsoleIO::GetNextChar(string prompt, std::vector<char> const& choices)

Also, you have a bug. You aren't ever deleteing buffer, which means it's leaking.

\$\endgroup\$
  • \$\begingroup\$ I appreciate the input, especially on the memory leak. Thanks! \$\endgroup\$ – Java Matrix Dec 6 '15 at 2:01
  • \$\begingroup\$ I suggest use of std::vector<char> const& choices. \$\endgroup\$ – R Sahu Dec 6 '15 at 8:30
  • \$\begingroup\$ @RSahu Excellent point! I'll update it. \$\endgroup\$ – user1118321 Dec 6 '15 at 15:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.