array loop not working correctly? c++

Question

Trying to count how many elements within the array are not equal to 0, is something set up wrong?

I'd like to check all values in the array (it's a sudoku board) and then when all elements are "full" I need to return true. Is something off?

bool boardFull(const Square board[BOARD_SIZE][BOARD_SIZE])
{
    int totalCount=0;
    for (int index1 = 0; index1 < BOARD_SIZE; index1++)
        for (int index2 = 0; index2 < BOARD_SIZE; index2++){ 
             if(board[index1][index2].number!=0)
                totalCount++;
        }
    if(totalCount=81)
        return true;
    else 
        return false;

I tempt to copy my answer in stackoverflow.com/questions/2563553/… here. — kennytm
– kennytm, Commented Apr 1, 2010 at 21:08
Why using constants for dimension and hardcoded 81 value ? isn't it meant to be BOARD_SIZE * BOARD_SIZE ? You've got a magic value here :p — Matthieu M.
– Matthieu M., Commented Apr 2, 2010 at 7:37

Matt · Accepted Answer · 2010-04-01 21:07:49Z

12

You have = rather than ==

if (totalCount == 81)

is the correct line.

Doing this with a single "=" actually assigns the value 81 to totalCount, so your test is essentialy:

if (81)

And since in C++ anything nonzero is true, this is always true

answered Apr 1, 2010 at 21:07

Matt

2,0552 gold badges17 silver badges15 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

codefail Over a year ago

I've been staring at this for too long. Thanks, now I feel stupid.

pm100 Over a year ago

get into the habit of compiling with high levels of warnings turned on - the compiler would have spotted this for you

John Dibling Over a year ago

Or write it like this: if( 81 == totalCount ) then it doesnt matter what warning level you are on.

kennytm Over a year ago

@John: Until you have if (var_a = var_b) where only warnings can save you.

Carl Norum · Accepted Answer · 2010-04-01 21:08:42Z

1

You have a = that should be a ==. That's all I'll say, since it's homework.

Also, why do you have a constant for BOARD_SIZE, then check against 81 at the end? Wouldn't checking against BOARD_SIZE * BOARD_SIZE be better?

answered Apr 1, 2010 at 21:08

Carl Norum

226k42 gold badges442 silver badges480 bronze badges

2 Comments

codefail Over a year ago

I guess that works as well, but in this case BOARD_SIZE does not change (declared as const elsewhere) so it's okay (albeit bad programming?)

Carl Norum Over a year ago

@igor, sure, but if it doesn't change why don't you have 9 everywhere, that's less typing than BOARD_SIZE if you don't care about it being a constant.

Chris O · Accepted Answer · 2010-04-01 21:09:19Z

0

Is the If(totalCount=81) a typo in this post or your code? Looks like you've assigned the value there.

answered Apr 1, 2010 at 21:09

Chris O

5,0373 gold badges38 silver badges42 bronze badges

Comments

fredoverflow · Accepted Answer · 2010-04-02 09:03:37Z

You can leave the function as soon as you find the first 0, and it's possible to solve this with a single loop:

bool boardFull(const Square board[BOARD_SIZE][BOARD_SIZE])
{
    const Square* p = board[0];
    for (int n = BOARD_SIZE * BOARD_SIZE; n; --n, ++p)
    {
        if (p->number == 0) return false;
    }
    return true;
}

But I prefer algorithms to hand-written loops:

struct Square
{
    int number;

    bool is_zero() const
    {
        return number == 0;
    }
};

#include <algorithm>
#include <functional>

bool boardFull(const Square board[BOARD_SIZE][BOARD_SIZE])
{
    return std::find_if(
        board[0],
        board[BOARD_SIZE],
        std::mem_fun_ref(&Square::is_zero)
    )== board[BOARD_SIZE];
}

Collectives™ on Stack Overflow

array loop not working correctly? c++

4 Answers 4

4 Comments

2 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

4 Comments

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related