0

I'm having trouble with something that should seem so simple. I'm using a conditional and on a pretest loop While and it doesn't seem to even execute one because the conditions are met but they're not.

I have but the loop never seems be met/ when I break on it. It just skips over it

int sorter = random.Next(0, 10);
bool player1full = false;
bool player2full = false;

while (player1full && player2full == false)
{
    if (chuckcards[sorter] != null)
    {
         while (player1full != true)
         {
              if (player1.Count != 5)
              {
                   player1.Enqueue(chuckcards[sorter]);
                   chuckcards[sorter] = null;
              }
              else
              {
                   player1full = true;
              }

              sorter = random.Next(0, 10);
         }

         while (player2full != true)
         {
              if (chuckcards[sorter] != null)
              {
                   if (player2.Count != 5)
                   {
                        player2.Enqueue(chuckcards[sorter]);
                        chuckcards[sorter] = null;
                   }
                   else
                   {
                        player2full = true;
                   }

                   sorter = random.Next(0, 10);
               }
          }
     }
     else
     {
          sorter = random.Next(0, 10);
     }
}

My logic maybe slightly off and I'm just wanting someone to point me in the right direction/see my error.

Thank you

Improve this question
6
  • 2
    since everybody has already commented on the while condition, i would also like to add that the code itself needs to be refactored into several small functions, you have way too much nesting and repetition Commented May 1, 2013 at 14:43
  • if i have the time I would. I'l probably clean it up before I put it on my github though :) Thanks for the suggestion Commented May 1, 2013 at 14:46
  • 1
    The thing about having those several small functions is actually about saving you time, not just cleaning up the code. Work smart from the beginning, and you won't need to refactor much along the way. And don't post code on github if you're only learning that ! is negate... Commented May 1, 2013 at 14:47
  • Further to @YoryeNathan point, it's also about being able to share code! If you stick everything into 1 big method, you may find it very hard to keep your code DRY Commented May 1, 2013 at 14:48
  • 1
    @DaveRook Reusability into readability into maintainability into extendability into saved time. Commented May 1, 2013 at 14:55

5 Answers 5

Reset to default
8

It will never enter the loop because here:

bool player1full = false;
bool player2full = false;
while (player1full && player2full == false)

This will test the Boolean value of player1full, and if it's true, then test the Boolean value of player2full == false. Since player1full is false, it stops right there and never enters the loop. I think what you want is:

while (player1full == false && player2full == false)

Or equivalently

while (!player1full && !player2full)

Or even (by De Morgan's Law):

while (!(player1full || player2full))

However, it seems like the entire outer loop is unnecessary. I can't be entirely sure without known the full context of your program (and that's out of scope for this question), but it could be rewritten as:

int sorter;
while (player1.Count != 5)
{
    sorter = random.Next(0, 10);
    if (chuckcards[sorter] != null)
    {
        player1.Enqueue(chuckcards[sorter]);
        chuckcards[sorter] = null;
    }
}

while (player2.Count != 5)
{
    sorter = random.Next(0, 10);
    if (chuckcards[sorter] != null)
    {
        player2.Enqueue(chuckcards[sorter]);
        chuckcards[sorter] = null;
    }
}
Improve this answer
Sign up to request clarification or add additional context in comments.

8 Comments

Yes. ! negates. Much cleaner. Just like != vs ==.
It is the not operator
@p.s.w.g how do you know his intention is to continue the loop only if both are false?
@DaxFohl I think it isn't. It's unclear, though. Overall, he wants shuffled cards dealt to players.
@DaxFohl New programmers often make the mistake of writing Boolean expressions as they would say them in English. In this case, it appears OP's intention was to write "if X and Y are false ..."--which is properly translated to "if X is false and Y is false"
|
3

The problem is here:

bool player1full = false;
bool player2full = false;
while (player1full && player2full == false)

that is not equivalent to 

while(player1full == false && player2full == false)

the == operator results in a new boolean, so thus the following is valid:

bool myBool = num1 == num2

What your condition essentially breaks down to is

(false && (false == false))

which reduces to

(false && true)

which reduces to

(false)
Improve this answer

5 Comments

do I have to check both then like while (player1full==false && player2full == false) ?
It seems you only want one, actually. That would be || instead of &&. Also, use ! instead of == false.
Thank you, Do I? I thought a conditional and because both have to be true both I move on. and I did try that originally but couldn't get it to work because I made the same mistake as above. Thanks
@Yop a && b only returns true when both a and b are true
@SamIam That what I want really because both will have to be size 5 before moving on.
2

It seems that everyone is thinking that you want to loop as long as both are false. But according to the logic, it looks to me that you only want one of them to be not-full.

If that is indeed the case, the condition should be !player1full || !player2full.

The reason I think that you might want only one to be not-full is that one may be full but the other one still needs handling. Not sure though. Depends on how you randomly distribute the cards to players, or whatever... And you seem to have edited that part out.

By the way, your shuffling method is horrible. Here is a neat and simple example:

List<string> cards = new List<string> { "1", "2", "3", ... , "10", "J", "Q", "K" };
List<string> shuffled = cards.Select(x => new { X = x, Y = random.Next() })
                             .OrderBy(x => x.Y)
                             .Select(x => x.X).ToList();

This isn't tested, since I don't have VS now, but the idea is to match every card to a random number, sort according to the random numbers, and then lose the extra information (that random number) so you have a shuffled list of cards.

So bottom line, you can just have a list of a player's cards, and shuffle it. Or you could have a full deck, shuffle, and take top 5 for each player. Whichever you choose, you want to shuffle them nicely.

Example for dealing cards after full-deck shuffling:

for (var i = 0; i < CARDS_PER_PLAYER; i++)
{
    foreach (var player in players)
    {
        // Shouldn't happen, if code is carefully planned
        if (cards.Count == 0) throw new ApplicationException("Out of cards!");

        // Deal the top card
        player.Enqueue(cards[0]);
        // Remove from deck, doh! Card can't be in deck AND in player's hand anyways
        cards.RemoveAt(0);
    }
}
Improve this answer

2 Comments

Thank you for this solution. I didn't know such shuffle ways existed and I just applied what method I thought was quickest and best at the time. I learn't how to shuffle my array from here: stackoverflow.com/questions/108819/… but this was useful. Thanks
Its basically the same shuffling method :)
1

This will never be true:

bool player1full = false;
bool player2full = false;
while (player1full && player2full == false)

Maybe you meant:

while (player1full == false && player2full == false)
Improve this answer

Comments

0

You should write :

while(!player1full && !player2full)

good luck!

Improve this answer

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.