While Loop And Condition C#

Question

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

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 — Alex, May 01 '13 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 — Yop, May 01 '13 at 14:46
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... — SimpleVar, May 01 '13 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](http://en.wikipedia.org/wiki/Don%27t_repeat_yourself) — Dave, May 01 '13 at 14:48
@Yop Very important question here--are you wanting to enter the loop if *either one* is false, or do you only want to enter the loop if *both* are false? — Dax Fohl, May 01 '13 at 14:54
@DaveRook Reusability into readability into maintainability into extendability into saved time. — SimpleVar, May 01 '13 at 14:55

p.s.w.g · Answer 1 · 2013-05-01T15:34:51.657

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;
    }
}

edited May 01 '13 at 15:34

answered May 01 '13 at 14:40

p.s.w.g

146,324
30
291
331

Yes. `!` negates. Much cleaner. Just like `!=` vs `==`. – SimpleVar May 01 '13 at 14:45
It is the [not operator](http://msdn.microsoft.com/en-US/library/f2kd6eb2%28v=vs.80%29.aspx) – Dave May 01 '13 at 14:46
@p.s.w.g how do you know his intention is to continue the loop only if *both* are false? – Dax Fohl May 01 '13 at 14:59
@DaxFohl I think it isn't. It's unclear, though. Overall, he wants shuffled cards dealt to players. – SimpleVar May 01 '13 at 15:02
1

@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*" – p.s.w.g May 01 '13 at 15:03
@p.s.w.g Meh, pay close attention to his code, and you'll see that the outter while loop and if are redundant anyways, and that he will almost definitely deal his players a few null cards. Don't only answer OP's question. Also answer OP's need. – SimpleVar May 01 '13 at 15:12
@YoryeNathan Well I've included an alternate solution. It does change the logic a little bit, but hopefully it "answers OP's need" :P – p.s.w.g May 01 '13 at 15:21
More sane, but still not too good as well. Check out my answer. And remove `int` from second while's sorter assignment. – SimpleVar May 01 '13 at 15:31

score 3 · Answer 2 · answered May 01 '13 at 14:39

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)

answered May 01 '13 at 14:39

Sam I am says Reinstate Monica

30,851
12
72
100

do I have to check both then like `while (player1full==false && player2full == false)` ? – Yop May 01 '13 at 14:40
It seems you only want one, actually. That would be `||` instead of `&&`. Also, use `!` instead of `== false`. – SimpleVar May 01 '13 at 14:41
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 May 01 '13 at 14:42
@Yop `a && b` only returns true when both `a` **and** `b` are true – Sam I am says Reinstate Monica May 01 '13 at 14:43
@SamIam That what I want really because both will have to be size 5 before moving on. – Yop May 01 '13 at 14:44

SimpleVar · Accepted Answer · 2013-05-01T15:08:33.047

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);
    }
}

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: http://stackoverflow.com/questions/108819/best-way-to-randomize-a-string-array-with-net but this was useful. Thanks — Yop, May 01 '13 at 20:48

score 1 · Answer 4 · answered May 01 '13 at 14:39

1

This will never be true:

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

Maybe you meant:

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

answered May 01 '13 at 14:39

gareththegeek

2,362
22
34

score 0 · Answer 5 · edited May 01 '13 at 14:49

0

You should write :

while(!player1full && !player2full)

good luck!

edited May 01 '13 at 14:49

Dax Fohl

10,654
6
46
90

answered May 01 '13 at 14:40

Obama

2,586
2
30
49

While Loop And Condition C#

5 Answers5