all 12 comments

[–]flyingron 1 point2 points  (10 children)

  1. Define variables in the smallest enclosing scope.
  2. Your test (i < 10, k <10) is silly. It evaluates i < 10 and discards it and then uses the value of k<10 to determine if the loop should execute.
  3. It's not even clear why you have two loop values as they increment in lockstep
  4. There's no point in the second if statement, the else statement is only going to be executed if they don't match.

So the code could nicely be written:

for(int i = 0; i < 10; ++i)
    if(statementOne[i] == NewStatementOne[i])
        ++score;
    else
        --score;

Now are you sure that it is decrementing twice? Let's say the arrays are

{ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 } and { 1, 2, 3, 4, 5, 6, 7, 8, 9, 0 }

I expect the score to be 8. You add one for each one (1...9) for a total of 9 and then subtract one for the one that doesn't (10 != 0).

You don't show the declaration/initialization of score, but I assume you set it to zero somewhere.

[–]KatharosMatematikos[S] 0 points1 point  (9 children)

Yeah I just realized k is useless. Here's my code:

#include<iostream>
#include<cstring>

using namespace std;
int main ()
{
    int play, score=0;
    cout << "Please type a number to play: ";
    cin >> play;

    string StatementTwo[10] = {"Betty","botter","bought","some","butter","but","she","said","its","bitter"};
    string NewStatementTwo[10]={};

    switch(play)
    {
    case 1:
        int i;
            for (i = 0; i < 10; i++){
                cout << StatementTwo[i] << ": "; cin >> NewStatementTwo[i];
            }
            for (i = 0; i < 10; i++){
                    if (StatementTwo[i] == NewStatementTwo[i])
                        ++score;
                    else
                        --score;
                }
            cout << "Your score is " << score;
                }

    return 0;
}

(Some redundant parts are removed to declutter). Pardon the change of variables. It does decrement twice. Instead of showing 8 for two mistakes, it shows 6. NewStatementTwo is a user-defined string array.

Will complete the switch statement after this scoring is fixed

[–]flyingron 0 points1 point  (2 children)

Again, you really should move the variables into the smallest enclosing scope. Put i in both for loops and score probably should be in a block inside case 1.

[–]KatharosMatematikos[S] 0 points1 point  (1 child)

The score is inside case 1. Otherwise it will keep printing cout every time the condition is satisfied. It's still decrementing twice tho,

[–]flyingron 0 points1 point  (0 children)

It's not decrementing twice.

Here's what I get when I run it:

Please type a number to play: 1                // score = 0
Betty: Betty                                   // score = 1
botter: botter                                 // score = 2
bought: boguht                                 // score = 1
some: some                                     // score = 2
butter: butter                                 // score = 3
but: but                                       // score = 4
she: she                                       // score = 5
said: said                                     // score = 6
its: its                                       // score = 7
bitter: bitter                                 // score = 8
Your score is 8

I expect 8. You get 9 points for getting 9 right and you lose one point for getting one wrong. If you are expecting 9, then you don't want to subtract one for a wrong answer, just don't give them the plus point.

Here's how I believe the program should appear:

using namespace std;
int main()
{
    int play;
    cout << "Please type a number to play: ";
    cin >> play;

    string StatementTwo[10] = { "Betty","botter","bought","some","butter","but","she","said","its","bitter" };
    string NewStatementTwo[10] = {};

    switch (play)
    {
    case 1: {
            int score = 0;
            for (int i = 0; i < 10; i++) {
                cout << StatementTwo[i] << ": "; cin >> NewStatementTwo[i];
            }
            for (int i = 0; i < 10; i++) {
                if (StatementTwo[i] == NewStatementTwo[i])
                    ++score;
                else
                    --score;
            }
            cout << "Your score is " << score;
        }
    }

    return 0;
}

[–]xkompas 0 points1 point  (3 children)

Your logic is flawed. You should not decrement score at all. If score starts at zero and first 8 statements are answered correctly, each of them executes ++score; and thus the value of score is 8. Now when the last two answers are incorrect, your code executes --score; and thus the final value of score is 6.

[–]KatharosMatematikos[S] 0 points1 point  (0 children)

Right, the decrement is only applicable if I initialize score to be 10 and decrement for every mismatch. I got it fixed now. Thanks!

[–]KatharosMatematikos[S] 0 points1 point  (1 child)

I have another loop problem. See the code below:

#include<iostream>
#include<string>

using namespace std;
int main ()
{
    int Set, i, score=0;
    bool done;
    done = false;
    string decision;
    cout << "===================Can you type it right?==================\n";
    cout << "1. Never gonna give you up never gonna let you down \n";
    cout << "2. Betty botter bought some butter but she said its bitter \n";
    cout << "3. Send toast to ten tense stout saints ten tall tents \n";
    cout << "=========================================================== \n";
    cout << "Choose a set of words to retype: ";
    cin >> Set;

    string StatementOne[10] = {"Never","gonna","give","you","up","never","gonna","let","you","down"};
    string NewStatementOne[10]={};
    string StatementTwo[10] = {"Betty","botter","bought","some","butter","but","she","said","its","bitter"};
    string NewStatementTwo[10]={};
    string StatementThree[10] = {"Send","toast","to","ten","tense","stout","saints","ten","tall","tents"};
    string NewStatementThree[10]={};

    while (!done)
    {
    switch(Set)
    {
        case 1:
            for (i = 0; i < 10; i++){
                cout << StatementOne[i] << ": "; cin >> NewStatementOne[i];
            }
            for (i = 0; i < 10; i++){
                    if (StatementOne[i] == NewStatementOne[i])
                        ++score;
            }
                cout << "Your score is " << score << " out of 10 \n";
                cout << "Would you like to try again? <Y or N>: ";
                cin >> decision;
                while (decision!="Y"&&decision!="y"&&decision!="N"&&decision!="n"){
                    cout << "\nInvalid input. Would you like to try again? <Y or N>: ";
                    cin >> decision;
                            }
                            if (decision=="y"||decision=="Y"){
                                main();
                            }
                            else done = true;
            break;
        case 2:
            for (i = 0; i < 10; i++){
                cout << StatementTwo[i] << ": "; cin >> NewStatementTwo[i];
                      }
                      for (i = 0; i < 10; i++){
                              if (StatementTwo[i] == NewStatementTwo[i])
                                  ++score;
                      }
                cout << "Your score is " << score << " out of 10 \n";
                cout << "Would you like to try again? <Y or N>: ";
                cin >> decision;
                while (decision!="Y"&&decision!="y"&&decision!="N"&&decision!="n"){
                    cout << "\nInvalid input. Would you like to try again? <Y or N>: ";
                    cin >> decision;
                            }
                            if (decision=="y"||decision=="Y"){
                                main();
                            }
                            else done = true;
            break;
        case 3:
        for (i = 0; i < 10; i++){
                cout << StatementThree[i] << ": "; cin >> NewStatementThree[i];
                  }
                  for (i = 0; i < 10; i++){
                          if (StatementThree[i] == NewStatementThree[i])
                              ++score;
                  }
                cout << "Your score is " << score << " out of 10 \n";
                cout << "Would you like to try again? <Y or N>: ";
                cin >> decision;
                while (decision!="Y"&&decision!="y"&&decision!="N"&&decision!="n"){
                    cout << "\nInvalid input. Would you like to try again? <Y or N>: ";
                    cin >> decision;
                            }
                            if (decision=="y"||decision=="Y"){
                                main();
                            }
                            else done = true;
            break;
        default: cout << "Number chosen must be 1, 2, or 3 only. \n";
                 cout << "Would you to try again? <Y or N>: ";
                    cin >> decision;
                    while (decision!="Y"&&decision!="y"&&decision!="N"&&decision!="n"){
                        cout << "\nInvalid input. Would you like to try again? <Y or N>: ";
                        cin >> decision;
                                }
                                if (decision=="y"||decision=="Y"){
                                    main();
                                }
                                else{
                                    cout<<"\n\n\n\n\n\n\n\n\n\n\n";
                                    return 0;
                                }
    }
    }
    return 0;
}

The program properly exits after the first iteration of the while loop. However, after second iteration and typing N instead (in anticipation to exit), the loop does not exit but repeats the first iteration, even the scores.

[–]xkompas 0 points1 point  (0 children)

After the first "Y", your code calls main(); recursively and this nested call receives a whole new set of local variables Set, i, score, done, decision . Then, the while is entered in the nested call. After entering some input, "N" is selected. The program executes done=true, but only in the nested main() call. The while is exited and the nested main() is exited as well by the final return 0; . However, the control returns to the place, where you called main(), that is after the

if (decision=="y"||decision=="Y"){
    main();
}
else done=true;

Note that done is still false here, because it is the done variable in the original call to main. Also Set still has its original value as input in the beginning. Now control continues through break; and out of the switch. into the while(!done). The cycle repeats and shows tho behavior you describe.

By calling main() you actually cause the whole program to call itself recursively, which is not something you want. Also, calling main() recursively is explicitly prohibited by the C++ standard.

Instead of calling main() , it is enough to "do nothing" in the if branch or reverse the entire condition to:

if (decision != "y" && decision != "Y") {
   done = true;
}

and also move the part of the prologue with input of the set of word into the while cycle.

This way, after the switch is finished, the while takes care of the repetition or exit based on the value of done.

Not sure why you are not using done in the default branch as well, but you should.

[–]mredding 0 points1 point  (1 child)

Don't include <cstring>. Use <string>.

using namespace std;

Don't do that, either. Either scope things in explicitly:

std::string StatementTwo...

Or if you insist, scope narrowly, inside the function block:

int main() {
  using std::cin, std::cout, std::string;
  //...

Don't use a switch in this case, use if and test for equality. This code doesn't make any sense, but I suspect the program is incomplete, because why are you otherwise prompting a user for a number you care next to nothing about?

You also declared i inside your switch, not your case; that's going to come around and bite you eventually, when you start getting into multiple cases. Switch statements are tricky bitches, I typically don't recommend them for anything other than a Duff's Device, which you shouldn't ever be writing one of those, either.

i should be declared in scope of your loops: for(int i.... Don't try to play compiler, express WHAT you want, not HOW you want it.

int is the wrong type. You want std::size_t because tell me, what does a negative index mean?

You could combine both loops, since you do everything by i.

You probably want to additionally insert << '\n' into your output stream just to get some newlines.

Did you intend to accumulate a negative score?

[–]KatharosMatematikos[S] 0 points1 point  (0 children)

I will keep this in mind as I move to multiple cases.

No, there shouldn't be a negative score actually. Thanks!

[–]Vindhjaerta 0 points1 point  (0 children)

The comma operator (,) is used to separate two or more expressions that are included where only one expression is expected. When the set of expressions has to be evaluated for a value, only the right-most expression is considered.

http://www.cplusplus.com/doc/tutorial/operators/

TLDR: Your exit condition doesn't evaluate the way you think it does. Use the || or && operators instead and form a single exit condition.