I wrote a bitvector class (well, I modified the one in Ron Penton's DSFGP book). I just have a couple of questions. First, in his class he has a SetAll function to set each bit to 1. Basically, something like this: I rewrote this to support cells of any size by changing m_array[index] = 0xFFffFFff to m_array[index] = ~0. I'm wondering if there's some sort of reason he didn't do it that way. The books all about keeping things as reusable as possible, but in this case he's saying each cell /must/ be 32 bits for it to work! Is there something wrong undesireable with the way I'm doing it? My second question is, is this line too much to pack into one line of code? He has it split up into an if/else statement, but I'm getting accustomed to using the ternary operator for cases like this. If you saw that code would you be irritated that I didn't split it up, or is that clear enough? Like I said, just a couple of boring questions, but they're two things I'd like to know before I start developing bad habits.

sizeof(0).

Your ternary operator usage, in this instance, obscures intent. Remember, "Everyone knows that debugging is twice as hard as writing a program in the first place. So if you are as clever as you can be when you write it, how will you ever debug it?" (Brian W. Kernighan)

Quote:Original post by Oluseyi
sizeof(0).

Ah yes, I knew my way seemed a bit too obvious to be correct. I'm using an unsigned long int as data type for each cell, so I suppose creating a tempory ulong variable, setting it to zero, and then negating it will work better.

Quote:
Your ternary operator usage, in this instance, obscures intent.

I somewhat thought so. OK, I need to kick the "pack everything onto as few lines as possible" habit now before I start entering the obsfucated code contests without trying. Not that that line comes anywhere close to being as bad as some of those entries, but I don't want to develop bad habits this early (or ever, really).

Thanks.

Quote:Original post by ontheheap

Quote:Original post by Oluseyi
sizeof(0).

Ah yes, I knew my way seemed a bit too obvious to be correct. I'm using an unsigned long int as data type for each cell, so I suppose creating a tempory ulong variable, setting it to zero, and then negating it will work better.

Or you could cast a literal 0. Or write '0L'.

Or (assuming C++), since you apparently want to support more than one cell size, use templating to handle the different cell sizes:

template <typename Cell>void SetAll(){   int index;   for( index = 0; index < m_size; index++)       m_array[index] = ~Cell(0);}

where you previously declared m_array as a Cell[].

Quote:

Quote:
Your ternary operator usage, in this instance, obscures intent.

I somewhat thought so. OK, I need to kick the "pack everything onto as few lines as possible" habit now before I start entering the obsfucated code contests without trying. Not that that line comes anywhere close to being as bad as some of those entries, but I don't want to develop bad habits this early (or ever, really).

Thanks.

Another problem is that writing things that way forces you to repeat the m_array[cell] 3 times instead of twice:

int flag = 1 << bit;if (p_value) m_array[cell] |= flag; // setelse m_array[cell] &= ~flag; // clear