difficulty with a piece of code, tic-tac-toe related

[Geminias]

how can that be though, i mean... the board has 9 values from 1 to 9, no value zero.

i initialized all values to 10 to begin with and i never check past the last choice.

[Fonzie]

smart move initializing them to 10, wish I'd done that to start with. Another probelm I encounted was that the same numbers were being added. like, count1, count2, and count3 were all equal to three. This means the program thinks you have the same space three times, therefore, making you win at times when you shouldn't. Could this be the problem?

[wtd]

Geminias wrote:

p.s. not having the ability to edit is just ridiculous.

c++:

#include <iostream> #include <stdlib.h>  //for rand() int check (int computer [], int player [],const int max); int check (int computer[], int player[],const int max) {      int checkC = 0, checkP = 0;                 for (int i = 0 ; i <= max  ; i++)  //this for loop adds 3 values in the array together to see if its 15  {          checkC = computer [i % max] + computer [(i + 1) % max ] + computer [(i + 2) % max ]  ;           checkP = player   [i % max] + player   [(i + 1) % max ] + player   [(i + 2) % max ]  ;                if (checkC == 15)       return (2);     else if (checkP == 15)       return (1); checkC = 0; checkP = 0;   }     if (max >= 4)   //since the for loop could get all but one possible combo, this loop is to check for the last possibility.   {          checkC = computer [0] + computer [2] + computer [3];       checkP = player   [0] + player   [2] + player   [3];   }            if (checkC == 15)           return (2);         else if (checkP == 15)           return (1);             return 0; }   int main () {        int board    [9] = {2,9,4,                      7,5,3,                      6,1,8};   int player   [5];   int computer [5];     int choice;      for (int i = 0 ; i < 5 ; i++)  //initializes everything to make sure all varables have value.  {         player   [i] = 10;      computer [i] = 10;          }         for (int i = 0 ; i < 5 ; i++)  {      std::cout << "Choose your next move: " << std::endl;      std::cin  >> choice;       player [i] = choice - 1;       std::cout << "Your choice was: " << choice << std::endl;    for (int s = 0 ; s <= i ; s++)    {       while ((computer [i] == 10) || (computer [i] == player [s]) )        {              computer [i] = rand() % 9;       }          }        std::cout << "Computer chooses: " << computer [i] << std::endl;     player   [i] = board [choice - 1];      computer [i] = board [computer [i]]; choice       = 0;             if ( i >= 2 )       choice = check(player, computer, i);  //choice is equal to the value check returns.  Note: Choice is used in two contexts.       if (choice != 0)          if (choice == 1)            std::cout << "YOU WIN!";          else if (choice == 2)            std::cout << "YOU LOSE!";        }     std::cout << "no one won!";     std::cin.get();  //chill when program finishes.         return 0; }

c++:

#include <iostream> // *** <stdlib.h> should be <cstdlib> // For access to rand(). #include <stdlib.h>  // *** Redundant forward declaration. int check (int computer [], int player [],const int max); int check(int computer[], int player[], const int max) {    int checkC = 0, checkP = 0;          // This for loop adds 3 values in the array together to    // see if it's 15.    for (int i = 0 ; i <= max; i++)     {        // *** You use i % max a lot.  Why not store that        // *** value in a variable with a meaningful name?        checkC = computer[i % max] + computer[(i + 1) % max ] + computer[(i + 2) % max ];        checkP = player[i % max]   + player[(i + 1) % max ]   + player[(i + 2) % max ];                   if (checkC == 15)           return 2;        else if (checkP == 15)           return 1;                  checkC = 0;        checkP = 0;    }      // *** This is not a loop, spo why does the comment say it is?    // Since the for loop could get all but one possible combo,    // this loop is to check for the last possibility.    if (max >= 4)       {          checkC = computer[0] + computer[2] + computer[3];       checkP = player[0]   + player[2]   + player[3];    }               if (checkC == 15)       return 2;    else if (checkP == 15)       return 1;          return 0; }   int main () {       int board[9] = {2,9,4                   ,7,5,3                   ,6,1,8},        player[5],        computer [5],        choice;      // Initializes everything to make sure all varables have value.    for (int i = 0 ; i < 5 ; i++)     {          player[i]   = 10;       computer[i] = 10;    }           for (int i = 0; i < 5; i++)    {       std::cout << "Choose your next move: " << std::endl;       std::cin  >> choice;             player[i] = choice - 1;       std::cout << "Your choice was: " << choice << std::endl;             for (int s = 0 ; s <= i ; s++)       {          while ((computer[i] == 10) || (computer[i] == player[s]))          {             computer[i] = rand() % 9;          }       }           std::cout << "Computer chooses: " << computer[i] << std::endl;           player[i]   = board[choice - 1];            computer[i] = board[computer[i]];       choice      = 0;         if ( i >= 2 )          // Choice is equal to the value check returns.           // Note: Choice is used in two contexts.          choice = check(player, computer, i);               // *** Nesting ifs without using braces is an exceptionally       // *** bad idea.  Is it:       // *** if (choice != 0)       // ***    if (choice == 1)       // ***       std::cout << "YOU WIN!";       // ***    else if (choice == 2)       // ***       std::cout << "YOU LOSE!";       // *** Or?       // *** if (choice != 0)       // ***    if (choice == 1)       // ***       std::cout << "YOU WIN!";       // *** else if (choice == 2)       // ***    std::cout << "YOU LOSE!";             if (choice != 0)          if (choice == 1)             std::cout << "YOU WIN!";          else if (choice == 2)             std::cout << "YOU LOSE!";           }         // *** So, after possibly printing a message about something     // *** winning or losing, you just have a blanket statement     // *** that no one won?     std::cout << "no one won!";     std::cin.get();  //chill when program finishes.         return 0; }

[bugzpodder]

looking at ur code briefly, I've noticed that you distinguished between a player and a computer. they are the same thing really (for example, when checking winning conditions). you should just keep them in an array together. also then you can allow the computer to go first with minimal complications.

[Geminias]

is this okay now wtd? Remark: By default if you choose 5 values it will display the message "no one won". (i'd like to make it end the program in the even that someone wins, any tips on the best way to do this?)

Also there is a bug in this program, which i know how to fix but can't spend the time to at the moment. It is the fact that both player and computer get a turn each round, so there could be a tie, and in the event of a tie, the computer would win by default.

i left your comments in there wtd, and posted some of my own (//*&*)

c++:

#include <iostream> // *** <stdlib.h> should be <cstdlib> // For access to rand(). #include <cstdlib>  // *** Redundant forward declaration.  // *&* Excuse me?  You mean to say that a function prototype isn't required? int check (int computer [], int player [],const int max); int check(int computer[], int player[], const int max) {    int checkC = 0, checkP = 0;       // This for loop adds 3 values in the array together to    // see if it's 15.    for (int i = 0 ; i <= max; i++)     {        // *** You use i % max a lot.  Why not store that        // *** value in a variable with a meaningful name?        // *&* look again, i use 'i % 4' only twice.                checkC = computer[i % 4] + computer[(i + 1) % 4 ] + computer[(i + 2) % 4 ];        checkP = player[i % 4]   + player[(i + 1) % 4 ]   + player[(i + 2) % 4 ];                   if (checkC == 15)           return 2;        if (checkP == 15)           return 1;                  checkC = 0;        checkP = 0;    }      // *** This is not a loop, spo why does the comment say it is?    // Since the for loop could get all but one possible combo,    // this for loop will check the last possibility.  // *&* oops, i meant if.    if (max >= 4)       {          checkC = computer[0] + computer[2] + computer[3];       checkP = player[0]   + player[2]   + player[3];    }               if (checkC == 15)       return 2;    if (checkP == 15)       return 1;          return 0; }   int main () {       int board[9] = {2,9,4                   ,7,5,3                   ,6,1,8};    int player[5];    int computer[5];    int choice;    bool OK;    // Initializes everything to make sure all varables have value.    for (int i = 0 ; i < 5 ; i++)     {          player[i]   = 10;       computer[i] = 10;    }           for (int i = 0; i < 5; i++)    {             OK = false;               std::cout << "Choose your next move: " << std::endl;       std::cin  >> choice;             player[i] = choice - 1;       std::cout << "Your choice was: " << choice << std::endl;             do       {          computer[i] = rand() % 9;          for (int s = 0 ; s <= i ; s++)          {              if  (computer[i] != player [s])              {                     if (i != s)                  {                      if (computer[i] != computer[s])                         OK = true;                      else                      {                         OK = false;                         break;                      }                       }                                                  else               {                 OK = false;                 break;               }               }             }          }        while (OK != false);                 std::cout << "Computer chooses: " << computer[i] << std::endl;           player[i]   = board[choice - 1];            computer[i] = board[computer[i]];       choice      = 0;         if ( i >= 2 )       {   // Choice is equal to the value check returns.           // Note: Choice is used in two contexts.          choice = check(computer, player, i);               // *** Nesting ifs without using braces is an exceptionally       // *** bad idea.  Is it:       // *** if (choice != 0)       // ***    if (choice == 1)       // ***       std::cout << "YOU WIN!";       // ***    else if (choice == 2)       // ***       std::cout << "YOU LOSE!";       // *** Or?       // *** if (choice != 0)       // ***    if (choice == 1)       // ***       std::cout << "YOU WIN!";       // *** else if (choice == 2)       // ***    std::cout << "YOU LOSE!";               if (choice != 0)         {                if (choice == 1)             {                std::cout << "YOU WIN!";                std::cin.ignore(1,'\n');                std::cin.ignore();             }             else if (choice == 2)             {             std::cout << "YOU LOSE!";             std::cin.ignore(1,'\n');             std::cin.ignore();             }         }       }     }         // *** So, after possibly printing a message about something     // *** winning or losing, you just have a blanket statement     // *** that no one won?  //*&* that is correct.  It's called a work in progress     std::cout << "no one won!";     std::cin.get();  //chill when program finishes.         return 0; }

[[Gandalf]]

Quote:

Excuse me? You mean to say that a function prototype isn't required?

A "function prototype" is only required by the compiler when you put your functions after your main(). Since you are placing your other functions before main(), the forward declarations are not neccessary, hence redundant.

[wtd]

code:

if (choice != 0)         {                if (choice == 1)             {                std::cout << "YOU WIN!";                std::cin.ignore(1,'\n');                std::cin.ignore();             }             else if (choice == 2)             {             std::cout << "YOU LOSE!";             std::cin.ignore(1,'\n');             std::cin.ignore();             }         }

Why do you have nested "if"s here? If "choice" is one or two, then it most certainly isn't zero.

[Geminias]

i didn't know that actually gandalf. That is informative, and wtd i'm disappointed... you were looking for bad code that hard to actually point out something as inconsequential as that?

sometimes i like to point out the obvious in my code because it helps organize things for a person (like myself who forgets the reason he wrote a line of code 10 seconds afterward). Besides, it is certainly not a mistake, it is certainly not inefficient in terms of processing speeds (if anything wouldn't it speed up processing by a few femto seconds? I can't answer this for myself since i don't know exactly how the processor deals with if statements, but it would seem logical to me that it tests each condition one by one, until it matches a condition.

Ergo, allowing it to find a match for one condition and if the match fails ignore the rest of the conditions would save on the total amount of work done.

Am i not correct? And if i am, note that this did not in any way affect my initial decision to use this technique, i merely mention it to emphasize the utter inconsequence this matter pertains. So why wtd, would you be mentioning it? unless i'm wrong of course... (which i'm not) lol

[wtd]

Geminias wrote:

i didn't know that actually gandalf. That is informative, and wtd i'm disappointed... you were looking for bad code that hard to actually point out something as inconsequential as that?

Is it inconsequential?

Such code makes me think the program was thrown together rather than designed.

[Geminias]

you draw up a design every time before you code?


maybe its bad practice, but one thing i could never do is design a relatively easy logic like this on paper when i can do it in my head and give my brain a work out. that's just me.

[wtd]

Not necessarily on paper, but I do stop and think before I start typing: what do I know about the problem?

http://www.compsci.ca/v2/viewtopic.php?p=101734#101734

[Geminias]

yeah i'd never do that, actually stop and think... lol


actually when i write code it doesnt usually just flow out in one linear blob that is flawless. Usually i have to go back and make changes and add things and subtract things, you know how it is. In this case i probably started the first if statement with (!= 0) before i made the function to return specific values according to who won. Then, when i added the other stuff in it didnt occur to me to remove the initial statement.

Personally, its like i say, you are just being overly picky in this instance.

But just so we are clear about the whole thing. No i'm not so stupid that i thought to myself "Hey first i'll check to see if its equal to zero, then i'll check to see if its equal to 1 and then 2."

No, my logic is better tuned than that. it was just something that got lost in the process as i came back and redid certain things and never had the need to remove the first line.

[md]

Geminias wrote:

yeah i'd never do that, actually stop and think... lol

actually when i write code it doesnt usually just flow out in one linear blob that is flawless. Usually i have to go back and make changes and add things and subtract things, you know how it is. In this case i probably started the first if statement with (!= 0) before i made the function to return specific values according to who won. Then, when i added the other stuff in it didnt occur to me to remove the initial statement.

Stopping and thinking is what makes someone a good programmer, as opposed to a mediocre programmer. If you don't think about what you are doing how are you ever going to tackle something as complex as say encryption, or networking, or complex graphics? Even something as simple as evaluating a function contained in a string (ie. "f(x)=x^2; f(2)") is almost impossible if you don't think about it.

After a certain point you have to plan things out or you'll just get lost and nothing will work. And it's not being "overly picky" to point out flaws, no matter how small it may be; easy flaw you don't fix is a flaw that will lead to eventual problems. Wtd is just helping to point them out to you.


*** editing posts really needs to be allowed... I had to delete my post to fix this... all because I pressed submit instead of preview by accident...

[Geminias]

cornflakes cornflakes... what is a man like me to do when he gets a response like that?? haha good lord man obviously i'm just kidding. Yeah hmm.. a programmer who can't stop and think.. hmmm.. did you not see the code i posted? You ever see a 2 year old who can't stop and think draw something like that up? I dont think i need to elaborate further...

i said that because wtd claims 'he stops and thinks' about a program which is why he'd never make a mistake like that.

and is it seriously considered a flaw? i keep hearing "pointed out flaw", what flaw? i dont see a flaw personally. dont know what you call it but its not a flaw.

[wtd]

Geminias wrote:

and is it seriously considered a flaw?

Yes it is. Any program that is more complex than it has to be to accomplish its intended purpose is flawed.

Get used to accepting constructive criticism, especially from people who know more about the subject than you do. There are lots of people who are way smarter than me, and I got to be as good as I am by listening to them, and observing how they code.

Note: complexity can have varying forms.