Fixing uninitialized local variable error

Question

I am working on a project right now and when I try to run what I have below it gives me an error that says "uninitialized local variable 'userOption' used" on line 22, while (isValidOption(userOption) == true) {. How do I fix that error? Thank you.

#include<iostream>
#include <string>
using namespace std;

char toupper(char ch) {
    if (ch >= 'A'&&ch <= 'Z')
        return(ch);
    else
        return(ch - 32);
}

bool isValidOption(char ch) {
    if (ch == 'I' || ch == 'O' || ch == 'L' || ch == 'X')
        return(true);
    else
        return(false);
}

char getMainOption() {
    string UserInput;
    char userOption;

    while (isValidOption(userOption) == true) {
        cout << "Choose One of the following options\n";
        cout << "I--List Our Inventory\n";
        cout << "O--Make an Order\n";
        cout << "L--List all Orders made\n";
        cout << "X--Exit\n";
        cout << "Enter an option: ";
        getline(cin, UserInput);
        userOption = toupper(UserInput[0]);

        if (!isValidOption(userOption)) {
            cout << "Invalid String\n";
            cout << "Enter an option: ";
            getline(cin, UserInput);
            userOption = toupper(UserInput[0]);
        }
        if (userOption == 'I')
            cout << "Listing Our Inventory\n";
        else if (userOption == 'O')
            cout << "Make an order\n";
        else if (userOption == 'L')
            cout << "Listing all orders\n";
    }
    return userOption;
}    

int main() {
    char choice;
    choice = getMainOption();

    system("pause");
    return 0;
}

Do something like: char userOption = ' '; Use a value that will pass your isValidOption() test or maybe use a do while loop — Marker, Oct 21 '18 at 18:55
You should rethink your `while` loop: condition `isValidOption(userOption) == true` should be checked **after** you get user input. And, probably, it should be `isValidOption(userOption) != true`. — Evg, Oct 21 '18 at 18:56

score 1 · Accepted Answer · answered Oct 21 '18 at 19:02

What the error is saying that you're trying to read from userOption before you've ever written to it. If a variable is uninitialized, its memory contents will be full of junk left behind by other functions and it can easily cause bugs. In your case, you'll want to read input from the user into userOption before you do any logic on it. This can be done with a do-while loop:

char userOption; // not yet initialized
do {
    ...
    cin >> userOption; // userOption gets initialized here on first loop run
} while (isValidOption(userOption)); // no need for == true, that's a tautology :-)
// NOTE: perhaps you want to loop while the input is INvalid, as in
// while (!isValidOption(userOption)); ?

A couply code-review comments I would additionally give are:

std::toupper already exists in <cctype>. Docs are here
return is not a function call and it's better to write return ch; than return(ch);
if (ch == 'I' || ch == 'O' || ch == 'L' || ch == 'X'){ return true; } else { return false; } is completely equivalent to the shorter return ch == 'I' || ch == 'O' || ch == 'L' || ch == 'X';
Also take a look at system(“pause”); - Why is it wrong?

Happy coding! Let me know if questions remain

Fixing uninitialized local variable error

1 Answers1