What is wrong with this code (C++)

[The_Doctor]

For a peice of coursework for my uni course I have to make a simple game.

I not very experienced with C++.

There are 3 difficulty levels in it, each one inside a loop which will contain the rest of the game. In this code I am testing it to see if the loops work.


#include <iostream.h>

int main()
{
bool Easy;
bool Medium;
bool Hard;
int Option;

std::cout<< "Game intructions\n";
std::cout<< "Please select a difficulty level\n";
std::cout<< "1=Easy\n";
std::cout<< "2=Medium\n";
std::cout<< "3=Hard\n";
std::cout<< "4=Quit\n";
std::cin>> Option;

if (Option == 1) {
Easy = true;
}

if (Option == 2) {
Medium = true;
}

if (Option == 3) {
Hard = true;
}

if (Option == 4) {
return 0;
}

while (Easy == true);
{
cout<< "Easy test works\n";
Easy = false;
}

while (Medium == true);
{
cout<< "Medium test works\n";
Medium = false;
}

while (Hard == true);
{
cout<< "Hard test works\n";
Hard = false;
}
return 0;
}

When I press 1 nothing happens.
When I press 2 it says "Easy Test Works".
When I press 3 it says "Easy Test Works", then "Medium Test Works".
Pressing 4 does end the program.

I have no idea way this is happening.

[doc_bean]

Try initiating your booleans (all false)

Have you tried using indirect input ? ( somthing like stringstream and then extracting)

Do you have to use while loops ? I'd use a case structure here.

I might be missing something obvious though, but I can't spot an obvious mistake.

EDIT: I'd also make use of only 2 booleans, if it isn't one or the other it has to be the thrid state, you might want to include an exception (or default) for the fourth state though.

[Husar]

I'd also get rid of the booleans and simply use the number in the option variable to determine the difficulty.
Can't say a lot about the rest, looks ok to me, but I never learned C++.

[Blodrast]

get rid of the ";" at the end of the while instructions;
i.e., instead of

while () ;
{

}


you should probably use

while () {

}

[Blodrast]

one more thing: if you're also being marked for style, it's poor style to use numerical constants in your code; if I were you, I'd replace the comparisons "if option == 2" with something like:

#define MODERATE_LEVEL 2

if (option == MODERATE_LEVEL) ...

And the same for the other options, naturally.

my two cents.

[The_Doctor]

I changed around some of it, and now it does not work. When you enter something and press enter it ends the program.:dizzy2:


#include <iostream.h>

void main()
{
#define Easy_Level 1
#define Medium_Level 2
#define Hard_Level 3
int Option;
bool Word_Complete;

std::cout<< "Game intructions\n";
std::cout<< "Please select a difficulty level\n";
std::cout<< "1=Easy\n";
std::cout<< "2=Medium\n";
std::cout<< "3=Hard\n";
std::cin>> Option;

if (Option == Easy_Level) {
do
{cout<< "Display scambled word\n";
Word_Complete = true;
}
while(Word_Complete = false);
}

if (Option == Medium_Level) {
do
{cout<< "Display scambled word\n";
Word_Complete = true;
}
while(Word_Complete = false);
}

if (Option == Hard_Level) {
do
{cout<< "Display scambled word\n";
Word_Complete = true;
}
while(Word_Complete = false);
}

return 0;
}

[Blodrast]

Again, I believe you're doing something that you're not intending to do:
in your while statements, I am guessing you want to test the condition, not to assign false to the boolean variable.
Hence, it should probably be
while (Word_Complete == false)

and NOT

while (Word_Complete = false)

Notice the difference ? ( == instead of =).
Also, in my previous comment, with regard to style, named constants should be in all caps. Like I said, EASY_LEVEL, not Easy_Level. It's just a convention, but that's how things are done. Again, if you're not getting any marks for style, you may as well not bother, although it's nice in general to have readable code.

[Blodrast]

One more thing: you're using the Word_Complete variable uninitialized.
Turn on warnings in your compiler (-Wall if you're using gcc/g++), it will notify you about it.

[The_Doctor]

I did what you advised. It did not work. I also tried it with a case/switch statement.


#include <iostream.h>

void main()
{
#define EASY_LEVEL 1
#define MEDIUM_LEVEL 2
#define HARD_LEVEL 3
int Option;
bool Word_Complete = false;

std::cout<< "Game intructions\n";
std::cout<< "Please select a difficulty level\n";
std::cout<< "1=Easy\n";
std::cout<< "2=Medium\n";
std::cout<< "3=Hard\n";
std::cin>> Option;

switch (Option)
{
case EASY_LEVEL:
do
{cout<< "Display scambled word Easy\n";
Word_Complete = true;
}
while(Word_Complete == false);
break;
case MEDIUM_LEVEL:
do
{cout<< "Display scambled word Medium\n";
Word_Complete = true;
}
while(Word_Complete == false);
break;
case HARD_LEVEL:
do
{cout<< "Display scambled word Hard\n";
Word_Complete = true;
}
while(Word_Complete == false);
break;
default:
cout<< "???";
}
return 0;
}

OR


#include <iostream.h>

void main2()
{
#define EASY_LEVEL 1
#define MEDIUM_LEVEL 2
#define HARD_LEVEL 3
int Option;
bool Word_Complete = false;

std::cout<< "Game intructions\n";
std::cout<< "Please select a difficulty level\n";
std::cout<< "1=Easy\n";
std::cout<< "2=Medium\n";
std::cout<< "3=Hard\n";
std::cin>> Option;

if (Option == EASY_LEVEL) {
do
{cout<< "Display scambled word Easy\n";
Word_Complete = true;
}
while(Word_Complete == false);
}

if (Option == MEDIUM_LEVEL) {
do
{cout<< "Display scambled word Medium\n";
Word_Complete = true;
}
while(Word_Complete == false);
}

if (Option == HARD_LEVEL) {
do
{cout<< "Display scambled word Hard\n";
Word_Complete = true;
}
while(Word_Complete == false);
}

}

I am using Dev-C++ version 4.

[Sigurd]

I am not sure what you are trying to do here doc...
but if you want a hotfix:

{

#define EASY_LEVEL 1
#define MEDIUM_LEVEL 2
#define HARD_LEVEL 3
#define END 4

int Option;
bool Word_Complete = false;

std::cout<< "Game intructions\n";
std::cout<< "Please select a difficulty level\n";
std::cout<< "1=Easy\n";
std::cout<< "2=Medium\n";
std::cout<< "3=Hard\n";


do
{
std::cin>> Option;

if (Option == EASY_LEVEL)
{
std::cout<< "Display scambled word Easy\n";
Word_Complete = true;
}

else if (Option == MEDIUM_LEVEL)
{
std::cout<< "Display scambled word Medium\n";
Word_Complete = true;
}

else if (Option == HARD_LEVEL)
{
std::cout<< "Display scambled word Hard\n";
Word_Complete = true;
}

else
{
return 0;
}
}
while (Option != END);
}

[LeftEyeNine]

Wow, looking at these, I feel like a CEO of a very serious corporation which is dedicated to conquer the world.

Keep on guys, I'll pump the wages :smoking:

[Husar]

That code looks pretty easy, I should try C++, it's to Java like German is to Dutch or so.
Also I noticed there is a fault here:


std::cout<< "1=Easy\n";
std::cout<< "2=Medium\n";
std::cout<< "3=Hard\n";
std::cin>> Option;

if (Option == EASY_LEVEL)

in the if-clause you check whether option is EASY_LEVEL, but since option is an integer, it cannot be a string and besides that, option was only assigned a number so that can never ever return true.
You either need to use a different check or turn option into a string and add three more lines and another variable and....


Despite Blodrast saying this was bad style, I would use


if (Option == 1)

Or you could change the start to say:


#include <string>

int Option;
string difficulty;
bool Word_Complete = false;

std::cout<< "Game intructions\n";
std::cout<< "Please select a difficulty level\n";
std::cout<< "1=Easy\n";
std::cout<< "2=Medium\n";
std::cout<< "3=Hard\n";
std::cin>> Option;

if (option == 1) {
difficulty = "EASY_LEVEL"}

if (option == 2) {
difficulty = "MEDIUM_LEVEL"}

if (option == 3) {
difficulty = "HARD_LEVEL"}

the three ifs could also be a case-option but then you could later check with


if (difficulty == "EASY_LEVEL")
{
std::cout<< "Display scambled word Easy\n";
}
but in Java checking strings does not work like this IIRC, so the condition may need a big change.
Ok, guess this really needs something else to check what is inside the string, but I would actually prefer numbers for such a small program anyway...

[Alexander the Pretty Good]

The_Doctor - the program ends after displaying the difficulty level because you haven't put anything after the switch statement that displays the difficulty level. If you want the program to continue after that, either add what else the code needs to do or put the entire thing in a loop as Sigurd Fafnesbane has done.

I would recommend using <iostream>instead of <iostream.h> and then put the phrase "using namespace std;" under it. Then you won't have to put std:: in front of every cout and cin. Just a little trick.

Husar - he's defined EASY_LEVEL to be 1, so the compiler treats every "EASY_LEVEL" as a 1. I guess Java doesn't have that feature...

[Husar]

Husar - he's defined EASY_LEVEL to be 1, so the compiler treats every "EASY_LEVEL" as a 1. I guess Java doesn't have that feature...
You're right, I didn't notice that.:2thumbsup:
At least my lazy brain got to think about some code again.~;)

[The_Doctor]

It works now.:beam:

I tried the code on one of the computers in university and it worked.

I wonder why it does not work on my computer?

[LeftEyeNine]

It works now.:beam:

I tried the code on one of the computers in university and it worked.

I wonder why it does not work on my computer?

So when are we moving on to conquest, boys? :smoking: Don't spoil my fun, hey!

[caravel]

Nothing appears to be wrong with that code in your first post apart from the booleans need to be initialised.

[KukriKhan]

So when are we moving on to conquest, boys? :smoking: Don't spoil my fun, hey!

LoL. I think you install his mini-game, and pick "Medium". No one ever picks medium. Then 'the plan' is revealed. :idea2:

[Blodrast]

Nothing appears to be wrong with that code in your first post apart from the booleans need to be initialised.

There is. He wants loops, but his while loops don't do anything, because they end in a semicolon. His code IS, both syntactically, and semantically, correct, yes, but I very much doubt that was what he wanted (while loops that do nothing), at least from what he explained.

[caravel]

There is. He wants loops, but his while loops don't do anything, because they end in a semicolon. His code IS, both syntactically, and semantically, correct, yes, but I very much doubt that was what he wanted (while loops that do nothing), at least from what he explained.

I've just spotted that! ~:doh: So without the semicolons and, if the booleans are initialised it should work? (bear in mind it has been a long time, and I am more C than C++ !) e.g.


#include <iostream.h>

int main()
{
bool Easy=false;
bool Medium=false;
bool Hard=false;
int Option=false;

std::cout<< "Game intructions\n";
std::cout<< "Please select a difficulty level\n";
std::cout<< "1=Easy\n";
std::cout<< "2=Medium\n";
std::cout<< "3=Hard\n";
std::cout<< "4=Quit\n";
std::cin>> Option;

if (Option == 1) {
Easy = true;
}

if (Option == 2) {
Medium = true;
}

if (Option == 3) {
Hard = true;
}

if (Option == 4) {
return 0;
}

while (Easy == true) {
cout<< "Easy test works\n";
Easy = false;
}

while (Medium == true) {
cout<< "Medium test works\n";
Medium = false;
}

while (Hard == true) {
cout<< "Hard test works\n";
Hard = false;
}
return 0;
}

??? :yes::no:

[Blodrast]

well, I'll be honest and I'll admit that I haven't actually tried the code - just looked at it.
But yeah, as far as I can tell, this should work just fine. Conceptually, it's correct, although it's always possible to miss a semicolon or some such.

[Sigurd]

I've just spotted that! ~:doh: So without the semicolons and, if the booleans are initialised it should work? (bear in mind it has been a long time, and I am more C than C++ !) e.g.


#include <iostream.h>

int main()
{
bool Easy=false;
bool Medium=false;
bool Hard=false;
int Option = 0;

std::cout<< "Game intructions\n";
std::cout<< "Please select a difficulty level\n";
std::cout<< "1=Easy\n";
std::cout<< "2=Medium\n";
std::cout<< "3=Hard\n";
std::cout<< "4=Quit\n";
std::cin>> Option;

if (Option == 1) {
Easy = true;
}

if (Option == 2) {
Medium = true;
}

if (Option == 3) {
Hard = true;
}

if (Option == 4) {
return 0;
}

while (Easy == true) {
cout<< "Easy test works\n";
Easy = false;
}

while (Medium == true) {
cout<< "Medium test works\n";
Medium = false;
}

while (Hard == true) {
cout<< "Hard test works\n";
Hard = false;
}
return 0;
}

??? :yes::no:

Ok... I have had a second look...
Manco Capac's code should work... if you initialise with a zero instead of false on the integer :beam: .

So, doc... you want to put a game in each of those loops?

[The_Doctor]

A few posts ago I did say I got it working in university.:yes:


So, doc... you want to put a game in each of those loops?

Yes. It is scrambled word solving game. The program picks a word out of a list and scrambles it. Then the player must de-scramble it, one letter at a time. I have all the code for this, it is just a matter of intergreating it into my code.

[Blodrast]

well, doc, let us know how it comes along :)
Sigurd: false IS zero ~;) - the wonders of C, eh ? :2thumbsup:

[The_Doctor]

well, doc, let us know how it comes along :)

Of course.:balloon2:

[The_Doctor]

I need more help, this time it is not the compiler that is causing the problems.

Here is the current code, I know it looks messy the final version will have comments in it.


#include <iostream.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <stdio.h>

void main()
{
#define EASY_LEVEL 1
#define MEDIUM_LEVEL 2
#define HARD_LEVEL 3
#define QUIT 4
int Option;
bool Word_Complete = false;
bool Run = true;
#define NUMBEROFWORDSEASY 9
#define MAXWORDLENGTHEASY 6
int randomnumber;
char wordeasy[NUMBEROFWORDSEASY][MAXWORDLENGTHEASY] = { "print", "phone", "worth", "throw", "games", "study", "error", "debug", "build"};
char myChosenWordEasy[MAXWORDLENGTHEASY];
char input;
int length;
char scrambledword[MAXWORDLENGTHEASY];
char answer[MAXWORDLENGTHEASY];
int WordsAnswered;
int Guess;
int WordCount;
int i;
bool gotrandomchar = false;

do {
cout<< "Game intructions\n";
cout<< "Please select a difficulty level\n";
cout<< "1=Easy\n";
cout<< "2=Medium\n";
cout<< "3=Hard\n";
cout<< "4=Quit\n";

cin>> Option;

switch (Option)
{
case EASY_LEVEL:
do
{srand(time(NULL));
randomnumber = rand()%NUMBEROFWORDSEASY;
strcpy(myChosenWordEasy, wordeasy[randomnumber]);
cout << "The random word is " << myChosenWordEasy << endl;

bool flag[MAXWORDLENGTHEASY-1];
for (i=0;i<MAXWORDLENGTHEASY-1;i++)
flag[i] = false;
myChosenWordEasy[MAXWORDLENGTHEASY];
length = MAXWORDLENGTHEASY-1;
for (i=0;i<MAXWORDLENGTHEASY-1;i++) {
bool gotrandomchar = false;
do {
int rndpos = rand()%length;
if (flag[rndpos]==false) {
flag[rndpos] = true;
scrambledword[i] = myChosenWordEasy[rndpos];
gotrandomchar = true;
}
}
while (gotrandomchar==false);
}
scrambledword[MAXWORDLENGTHEASY-1] = NULL;
cout << "The original word is " << myChosenWordEasy << "\n";
cout << "The scrambled word is " << scrambledword << "\n";

cin >> answer;
if (answer == myChosenWordEasy) {
cout<< "The word is correct\n";
WordCount = WordsAnswered + 1;
if (WordsAnswered == 3) {
cout << "Well done!!!\n";
Word_Complete = true;
}
}
else {
cout<< "The word is not correct\n";
Guess = Guess + 1;
if (Guess == 3) {
cout<< "You have failed\n";
Word_Complete = true;
}
}
}
while(Word_Complete == false);
break;

case MEDIUM_LEVEL:
cout<< "Display scambled word Medium\n";
do {
cout<< "Medium loop is working\n";
Word_Complete = true;
}
while(Word_Complete == false);
break;

case HARD_LEVEL:
cout<< "Display scambled word Hard\n";
do {
cout<< "Hard loop is working\n";
Word_Complete = true;
}
while(Word_Complete == false);
break;
case QUIT:
Run = false;

default:
cout<< "Press enter to end game\n";
}
}
while (Run == true);
}

Problem 1:
If I type in the correct answer for the scrambled word, it says it is incorrect. I think is something to do with answer and myChosenWordEasy being arrays.

I think this is wrong:

Guess = Guess + 1

If you can make sense of this and help, Thank You.:balloon2:

[Blodrast]

Hi Doc,

I have a few suggestions:

1. Initialize Guess. You shouldn't try to use uninitialized variables - and while we're at it, I'll remind you again to enable warnings in your compiler - using uninitialized variables should cause a warning.
Make sure you initialize all other variables before using them (note: here, by "use" I mean "read" from them, test them; if you first write to them before any use, then you don't need to initialize them).

2. You are right, you cannot compare two char arrays to figure out if two strings are identical. Use strcmp instead (man strcmp).

3. Hard to read unindented code (I'm assuming it's because of the way the post gets formatted by the board), but I'm not sure what this line does:

myChosenWordEasy[MAXWORDLENGTHEASY];

in the context:
for (i=0;i<MAXWORDLENGTHEASY-1;i++)
flag[i] = false;
myChosenWordEasy[MAXWORDLENGTHEASY];
length = MAXWORDLENGTHEASY-1;

Is that an incompletely pasted line ? The code shouldn't even compile with a line like that, as far as I can tell.

-----------

I really don't understand the second half of your post, so I can't make any comments on that.

hope this helps a bit.

[The_Doctor]

I'm not sure what this line does:

myChosenWordEasy[MAXWORDLENGTHEASY];

I copied and pasted the code that we where given and the start of the project and then modified it.

That line used to declare myChosenWordEasy as a variable, and since then I have moved it to the top of the code. It would seem that I had forgot to delete it.


Hard to read unindented code (I'm assuming it's because of the way the post gets formatted by the board)

Yes it is the board.


I really don't understand the second half of your post, so I can't make any comments on that.

Ignore that bit.


2. You are right, you cannot compare two char arrays to figure out if two strings are identical. Use strcmp instead (man strcmp).

That is very useful, thanks.

[The_Doctor]

Update:
The strcmp worked perfectly. Thank You Blodrast.:beam:


More questions:
1. How do I take a single letter out of a string and compare it to another letter in another string that only has one letter.

For example:
I have the word "games", then it is scrambled to "agmse". "agmse" appear on the screen and the user is asked to guess the first letter of the scrambled word. Then the user inputs "g" and then the program checks if the this matches the first letter in the original word. Then it asks the user to guess the second letter and check that, and so on until it gets to the end of the word.

I have a feeling that it will contain a for loop.

[Blodrast]

Say the word to be guessed is named wtbg.

guessed_chars_so_far = 0;
while (guessed_chars_so_far < strlen (wtbg)) {
boolean guessed = false;
while (guessed == false) {
cout << "Guess character in position " << guessed_chars_so_far << endl;
cin >> c;
if (c == wtbg[guessed_chars_so_far]) {
cout << "Correct !" << endl;
guessed = true;
guessed_chars_so_far ++;
}
else {
cout << "Wrong guess, let's try again " << endl;
}
}
}


To answer your question, to compare two characters you don't need strcmp, you can simply compare them (with ==). If you want to compare two strings, you need to use strcmp.

edit: even though my code was indented, the board ignores that...

[The_Doctor]

Great, I will try it tomorrow.:balloon2:

[Sigurd]

edit: even though my code was indented, the board ignores that

guessed_chars_so_far = 0;
while (guessed_chars_so_far < strlen (wtbg))


{
boolean guessed = false;
while (guessed == false)


{
cout << "Guess character in position " << guessed_chars_so_far << endl;
cin >> c;
if (c == wtbg[guessed_chars_so_far])
{


cout << "Correct !" << endl;
guessed = true;
guessed_chars_so_far ++;


}
else


{
cout << "Wrong guess, let's try again " << endl;


}


}


}



------------------------------------------------------------------
Like this? ... :sweatdrop:

[Blodrast]

gasp!
what is the secret ?! It took me longer to indent the stuff than it took to write the damn thing...

[The_Doctor]

Update:
It works!!!:beam: (well more or less)

There are a few bugs to work out, I should be able to work these out by myself, if I can't there is always you guys.:2thumbsup:

[Blodrast]

great, good to hear :)