Public Group

# Bit vector and coding style questions

This topic is 4889 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

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:
void SetAll()
{
int index;
for( index = 0; index < m_size; index++)
m_array[index] = 0xFFffFFff // he's using 32 bits per cell
}


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?
m_array[cell] = p_value ? (m_array[cell] | (1<< bit)):(m_array[cell] & (~(1<<bit)));


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.

##### Share on other sites
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)

##### Share on other sites
Quote:
 Original post by Oluseyisizeof(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.

##### Share on other sites
Quote:
Original post by ontheheap
Quote:
 Original post by Oluseyisizeof(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

• 10
• 17
• 9
• 14
• 41
• ### Forum Statistics

• Total Topics
631068
• Total Posts
2997740
×