3
\$\begingroup\$

My code currently generates a 2-variable truth table and lets the user select to AND/OR/NOT the variables. I was looking for advice on how to make it more concise, handle bad inputs better, and ignore case while going through.

CODE:

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

int main() {
bool p[4] = { true, true, false, false };
bool q[4] = { true, false, true, false };

cout << "Do you want to AND or OR the two propositional variables?" << endl;
string andor;
cin >> andor;
cout << "Do you want to NOT p? Y/N" << endl;
string ansp;
cin >> ansp;
cout << "Do you want to NOT q? Y/N" << endl;
string ansq;
cin >> ansq;

if (andor == "AND" || andor == "OR" &&
    ansq == "Y" || ansq == "N" &&
    ansp == "Y" || ansp == "N") {
    if (andor == "AND" && ansp == "N" && ansq == "N") {
        cout << "p | q" << " | " << "p A q" << endl;
        for (int i = 0; i < 4; i++) {
            cout << setw(1) << p[i] << " | ";
            for (int j = 0; j < 1; j++) {
                cout << setw(1) << q[i] << " | ";
                cout << setw(3) << (p[i] && q[i]);
            }
            cout << endl;
        }
    }
    else if (andor == "AND" && ansp == "Y" && ansq == "N") {
        cout << "p | q" << " | " << "~p A q" << endl;
        for (int i = 0; i < 4; i++) {
            cout << setw(1) << p[i] << " | ";
            for (int j = 0; j < 1; j++) {
                cout << setw(1) << q[i] << " | ";
                cout << setw(3) << (!(p[i]) && q[i]);
            }
            cout << endl;
        }
    }
    else if (andor == "AND" && ansp == "N" && ansq == "Y") {
        cout << "p | q" << " | " << "p A ~q" << endl;
        for (int i = 0; i < 4; i++) {
            cout << setw(1) << p[i] << " | ";
            for (int j = 0; j < 1; j++) {
                cout << setw(1) << q[i] << " | ";
                cout << setw(3) << (p[i] && !(q[i]));
            }
            cout << endl;
        }
    }
    else if (andor == "AND" && ansp == "Y" && ansq == "Y") {
        cout << "p | q" << " | " << "~p A ~q" << endl;
        for (int i = 0; i < 4; i++) {
            cout << setw(1) << p[i] << " | ";
            for (int j = 0; j < 1; j++) {
                cout << setw(1) << q[i] << " | ";
                cout << setw(3) << (!(p[i]) && !(q[i]));
            }
            cout << endl;
        }
    }
    else if (andor == "OR" && ansp == "N" && ansq == "N") {
        cout << "p | q" << " | " << "p V q" << endl;
        for (int i = 0; i < 4; i++) {
            cout << setw(1) << p[i] << " | ";
            for (int j = 0; j < 1; j++) {
                cout << setw(1) << q[i] << " | ";
                cout << setw(3) << (p[i] || q[i]);
            }
            cout << endl;
        }
    }
    else if (andor == "OR" && ansp == "Y" && ansq == "N") {
        cout << "p | q" << " | " << "~p V q" << endl;
        for (int i = 0; i < 4; i++) {
            cout << setw(1) << p[i] << " | ";
            for (int j = 0; j < 1; j++) {
                cout << setw(1) << q[i] << " | ";
                cout << setw(3) << (!(p[i]) || q[i]);
            }
            cout << endl;
        }
    }
    else if (andor == "OR" && ansp == "N" && ansq == "Y") {
        cout << "p | q" << " | " << "p V ~q" << endl;
        for (int i = 0; i < 4; i++) {
            cout << setw(1) << p[i] << " | ";
            for (int j = 0; j < 1; j++) {
                cout << setw(1) << q[i] << " | ";
                cout << setw(3) << (p[i] || !(q[i]));
            }
            cout << endl;
        }
    }
    else if (andor == "OR" && ansp == "Y" && ansq == "Y") {
        cout << "p | q" << " | " << "~p V ~q" << endl;
        for (int i = 0; i < 4; i++) {
            cout << setw(1) << p[i] << " | ";
            for (int j = 0; j < 1; j++) {
                cout << setw(1) << q[i] << " | ";
                cout << setw(3) << (!(p[i]) || !(q[i]));
            }
            cout << endl;
        }
    }       
}
else {
    cerr << "ERROR: Please enter valid values - EX(AND, OR, Y, N)." << endl;
}

return 0;
}
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! I hope you get some great answers. \$\endgroup\$
    – Phrancis
    Feb 29, 2016 at 20:57

2 Answers 2

1
\$\begingroup\$

To make it more concise you need to use functions. The reason I suggest that you use functions is because your repeat a lot of your code, and you don't have to. If you create functions that contain the code you repeat it will make your code smaller and easier to read, it's like writing chapters in a book or paragraphs in an essay. You repeat your for loops many times, if you use functions such as OutputAndTable() below and an OutputOrTable() function you only have 2 versions of the for loop (if you add the 2 arrays I added which are notp[] and notq[]). Some examples of functions you need are:

void OutputAndTable(bool *p, bool *q)
{
    cout << "p | q" << " | " << "p A q" << endl;
    for (int i = 0; i < LOGICARRAYSIZE; i++) {
        cout << setw(1) << p[i] << " | ";
        for (int j = 0; j < 1; j++) {
            cout << setw(1) << q[i] << " | " << setw(3) << (p[i] && q[i]);
        }
        cout << endl;
    }
}

void GetAnInput(const char *question, string &answer)
{
    cout << question << endl;
    cin >> answer;
    MyIgnoreCase(answer);
}

void GetAllInput(string &andor, string &ansp, string &ansq)
{
    GetAnInput("Do you want to AND or OR the two propositional variables?", andor);
    if ((andor != "AND") && (andor != "OR")) {
        cerr << "ERROR: Please enter a valid values - (AND or OR)." <<   endl;
    }
    GetAnInput("Do you want to NOT p? Y/N", ansp);
    GetAnInput("Do you want to NOT q? Y/N", ansq);
}

The function GetAllInput() show's how you can handle inputs better, although to truely handle inputs better you would want to loop until you get the correct answer.

To ignore case, choose your internal representation, either all capitals or all lower case and then convert the strings to your desired internal representation. The following function provides an example:

/**
 * Ignore the case of the input by making everything upper case.
 */
void MyIgnoreCase(string &answer)
{
    for (int i = 0; i < answer.size(); i++ ) {
        answer[i] = toupper(answer[i]);
    }
}

To make your code even more concise you should simplify your if statements:

    bool p[LOGICARRAYSIZE] = { true, true, false, false };
    bool q[LOGICARRAYSIZE] = { true, false, true, false };
    bool notp[LOGICARRAYSIZE];
    for (int i = 0; i < LOGICARRAYSIZE; i++ )
    {
        notp[i] = !p[i];
    }
    bool notq[LOGICARRAYSIZE];
    for (int i = 0; i < LOGICARRAYSIZE; i++ )
    {
        notq[i] = !q[i];
    }

    GetAllInput(andor, ansp, ansq);


    bool *PParameter;
    bool *QParameter;
    if (ansq == "Y") { // if Not q
       QParameter = notq;
    } else {
       QParameter = q;
    }
    if (ansp == "Y") {  // if Not P
       PParameter = notp;
    } else {
       PParameter = p;
    }
    if (andor == "AND") {
        OutputAndTable(PParameter, QParameter);
    }

You should also replace '4' with a constant that helps your reader understand your code better:

const int LOGICARRAYSIZE = 4;

References you should look at are http://en.cppreference.com/w/cpp/string/byte/toupper and http://en.cppreference.com/w/cpp/string/basic_string

\$\endgroup\$
0
\$\begingroup\$

You could and should read the user inputs into variables of types that represent what you are doing with them best.

E.g. whether the user wants to invert p or q should be a boolean. For the operation I would recommend declaring an enum.

I would also print messages about input errors directly where they happen. If a user already enters a wrong operation (e.g. "XOR") in the first question, why should you let him enter two additional values before you tell him. Additionally your messages can be more specific.

Checking/Comparing user input that is not required to be case sensitive is easiest done by making the user input upper case (or lower case) and compare against the wanted/required values in the same case. This method is also less frustrating if you don't need to retype Y as you accidentially typed y.

If the user enters something invalid you can also force him to enter a valid value by using a loop that doesn't quit until something valid was entered.

And last but not least you output. You can completely stream line the way you do it by "calculating" the values depending on the user input beforehand and output everything later.

E.g. this line:

cout << "p | q" << " | " << "p A q" << endl;

appears in 8 variations while you could make a single one out of it:

cout << "p | q" << " | " << (bInvertP?"~p":"p") << " " << (type==AND?"A":"V") << " " << (bInvertQ ? "~q" : "q") << endl;

The same way you can "prepare" the values of p and q. If the user wants p to be inverted you use p[i] otherwise you use !p[i] for both calculation and output.


#include <iostream>
#include <string>
#include <iomanip>
#include <algorithm>

using std::string;
using std::cin;
using std::cout;
using std::cerr;
using std::endl;
using std::setw;

enum Type
{
    AND,
    OR
};

string& toUpper(string& str)
{
    for (auto iter = str.begin(); iter != str.end(); ++iter) { *iter = toupper(*iter); } return str;
}

int main() 
{
    bool p[4] = { true, true, false, false };
    bool q[4] = { true, false, true, false };

    //Used to evaluate data input by user
    string userInput;
    bool bValidInput = false;

    //Parameters resulting from user input
    Type type = AND;
    bool bInvertP = false;
    bool bInvertQ = false;

    do
    {
        bValidInput = false;
        cout << "Do you want to AND or OR the two propositional variables?" << endl;
        cin >> userInput;  //Read input
        toUpper(userInput); //Make uppercase for easier comparison
        if (userInput == "AND")
        {
            type = AND; bValidInput = true;
        }
        else if (userInput == "OR")
        {
            type = OR; bValidInput = true;
        }
        else
        {
            cerr << "ERROR: Please enter valid values [AND, OR]" << endl;
        }
    } while (!bValidInput); //Force user to enter valid input

    do
    {
        bValidInput = false;
        cout << "Do you want to NOT p? Y/N" << endl;
        cin >> userInput;  //Read input
        toUpper(userInput); //Make uppercase for easier comparison
        if (userInput == "Y")
        {
            bInvertP = true; bValidInput = true;
        }
        else if (userInput == "N")
        {
            bInvertP = false; bValidInput = true;
        }
        else
        {
            cerr << "ERROR: Please enter valid values [Y, N]" << endl;
        }
    } while (!bValidInput); //Force user to enter valid input

    do
    {
        bValidInput = false;
        cout << "Do you want to NOT q? Y/N" << endl;
        cin >> userInput;  //Read input
        toUpper(userInput); //Make uppercase for easier comparison
        if (userInput == "Y")
        {
            bInvertQ = true; bValidInput = true;
        }
        else if (userInput == "N")
        {
            bInvertQ = false; bValidInput = true;
        }
        else
        {
            cerr << "ERROR: Please enter valid values [Y, N]" << endl;
        }
    } while (!bValidInput); //Force user to enter valid input

    //Use user params to build the information for the user
    cout << "p | q" << " | " << (bInvertP?"~p":"p") << " " << (type==AND?"A":"V") << " " << (bInvertQ ? "~q" : "q") << endl;
    for (int i = 0; i < 4; i++) 
    {
        bool pValue = bInvertP ? !p[i] : p[i]; //Value of p dependending on inversion
        for (int j = 0; j < 1; j++) 
        {
            bool qValue = bInvertQ ? !q[i] : q[i];//Value of q dependending on inversion
            bool result = type == AND ? (pValue && qValue) : (pValue || qValue); //Calculate result of wanted operation

            cout << setw(1) << pValue << " | " << setw(1) << qValue << " | " << setw(3) << result << endl; //Print the result
        }
    }

    return 0;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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