Jumping into c++ chapter 5 prob 7

Question

as recommended I've been working through the book 'Jumping into c++'. I'm currently on problem 7 of chapter 5 and although I have produced the code that appears to do what is asked of me I was hoping someone might be able to take a look and tell me if I've implemented any 'bad' practice (Ideally I don't want to be picking up bad habits already).

Secondly, it also says 'try making a bar graph that shows the results properly scaled to fit on your screen no matter how many results were entered'. Again, the code below produces a horizontal bar graph but I'm not convinced that if I had say 10000 entries (I guess I could verify this by adding an additional for loop) that it would scale according. How would one go about applying this? (such that it always properly scales regardless of how many entries).

I should probably point out at this point that I have not covered topics such as arrays, pointers and classes as of yet in case anyone was curious as to why I didn't just create a class called 'vote' or something.

One final thing... I don't have a 'return 0' in my code, is this a problem? I find it slightly confusing as to what exactly the point of having return 0 is. I know that it's to do with making sure your code is running properly but it seems sort of redundant?

Thanks in advance!

#include <iostream>

using namespace std;

int main()
{
    int option;
    int option_1 = 0;
    int option_2 = 0;
    int option_3 = 0;
    cout << "Which is your favourite sport?" << endl;
    cout << "Tennis.. 1" << endl;
    cout << "Football.. 2" << endl;
    cout << "Cricket.. 3" << endl;

    cin >> option;
while(option != 0)
{
    if(option == 1)
    {
        option_1++;
    }
    else if(option ==2)
    {
        option_2++;
    }
    else if(option ==3)
    {
        option_3++;
    }
    else if(option > 3 || option < 0)
    {
        cout << "Not a valid entry, please enter again" << endl;
    }
    else if(option ==0)
    {
        break;
    }
    cout << "Which is your favourite sport?" << endl;
    cout << "Tennis.. 1" << endl;
    cout << "Football.. 2" << endl;
    cout << "Cricket.. 3" << endl;
    cin >> option;
}
    cout << "Option 1 (" << option_1 << "): ";
    for(int i = 0; i < option_1; i++)
    {
        cout << "*";
    }
    cout << "" << endl;
    cout << "Option 2 (" << option_2 << "): ";
        for(int i = 0; i < option_2; i++)
    {
        cout << "*";
    }
    cout << "" << endl;
    cout << "Option 3 (" << option_3 << "): ";
        for(int i = 0; i < option_3; i++)
    {
        cout << "*";
    }


}

Answer 1

About the return 0 in main : it's optional in C++.

About your code:

• You have a ton of if / else if blocks, you should replace them with a switch. A switch statement is more compact, readable, and may be a little bit faster at runtime. It's not important at this point, but it's pretty good practice to know where to put a switch and where to use regular if.

• You have one big function, it's really bad. You should break your code into small, reusable pieces. That's something called DRY (Don't repeat Yourself): if you are copy-pasting code, you're doing something wrong. For example, your sport list appears 2 times in your code, you should move it in a separate function.

• You wrote cout << "" << endl;, I think you don't really understand how std::cout work. std::cout is an object representing the standard output of your program. You can use operator<< to pass values to this standard output. std::endl is one of these values you can pass, strings are, too. So you can just write cout << endl;, no need for an empty string.

• Please learn how to use arrays, either raw ones or std::array. This is a pretty good example of a program which can be refactored using arrays.

Here is a more readable, cleaner version of your code:

#include <iostream>

int prompt_option()
{
    int option;

    while (true)
    {
        std::cout << "Which is your favourite sport?" << std::endl;
        std::cout << "Tennis.. 1" << std::endl;
        std::cout << "Football.. 2" << std::endl;
        std::cout << "Cricket.. 3" << std::endl;
        std::cin >> option;
        if (option >= 0 && option <= 3)
            return option;
        else
            std::cout << "Not a valid entry, please enter again" << std::endl;
    }
}

void display_option(int number, int value)
{
    std::cout << "Option " << number << " (" << value << "): ";
    while (value--)
        std::cout << '*';
    std::cout << std::endl;
}

int main()
{
    int option;
    int values[3] = {0};

    while (true)
    {
        option = prompt_option();
        if (option)
            values[option - 1]++;
        else
            break;
    }
    for (int i = 0; i < 3; i++)
        display_option(i + 1, values[i]);
}

Answer 2

You have too much if else, it messy. check out the code bellow, muc shorter, cleaner and efficent. I hope it helps.

#include <iostream>

using namespace std;

int main()
{
    int choice;
    int soccer=0, NFL=0 ,formula1=0;
    while(choice != 0){
        cout<<"Please choose one of the following for the poll"<<endl;
        cout<<"press 1 for soccer, press 2 for NFL, press 3 for formula 1"<<endl;
        cout<<"Press 0 to exit"<<endl;
        cin>>choice;
        if(choice==1){
            soccer++;

        }
        else if(choice==2){
            NFL++;

        }
        else if(choice == 3){
            formula1++;

        }
        else{
            cout<<"Invalid entry, try again"<<endl;
        }
        cout<<"soccer chosen "<<soccer<<" times.";
        for(int i=0; i<soccer; i++){
            cout<<"*";
        }
        cout<<endl;

        cout<<"NFL chosen "<<NFL<<" times.";
        for(int j=0; j<NFL; j++){
            cout<<"*";
        }
        cout<<endl;
        cout<<"formula1 chosen "<<formula1<<" times.";
        for(int c=0; c<formula1; c++){
            cout<<"*";
        }
        cout<<endl;



    }

    return 0;
}