Conversation
|
|
||
| using namespace Poly; | ||
|
|
||
| constexpr BitMask::DataType TYPE_BIT = CHAR_BIT * sizeof(BitMask::DataType); |
There was a problem hiding this comment.
constexpr size_t TYPE_BIT = CHAR_BIT * sizeof(BitMask::DataType);
| if (size%TYPE_BIT) | ||
| arraySize = size / TYPE_BIT + 1; | ||
| else | ||
| arraySize = size / TYPE_BIT; |
There was a problem hiding this comment.
Change this whole block to arraySize = (size + TYPE_BIT - 1) / TYPE_BIT;
| BitMask operator|(const BitMask& rhs) const; | ||
| BitMask operator^(const BitMask& rhs) const; | ||
| BitMask operator&(const BitMask& rhs) const; | ||
| BitMask& operator~(); |
There was a problem hiding this comment.
This operator should be const and return a copy, just like &, | and ^.
| bool operator!=(const BitMask rhs) const | ||
| { | ||
| return !(*this == rhs); | ||
| } |
There was a problem hiding this comment.
Put this whole method in one line. This should accept const BitMask& as argument
| { | ||
| return !(*this == rhs); | ||
| } | ||
| bool operator[](size_t index) const; |
There was a problem hiding this comment.
Please add @todo annotation: "Convert this method to return bit proxy", or if you want I can tell you how to write this proxy. In general we want to be able to do sth like this: BitMask b; b[0] = true; which is not possible right now.
|
|
||
| BitMask BitMask::operator^(const BitMask& rhs) const | ||
| { | ||
| if (BitList.GetSize() == rhs.BitList.GetSize()) |
There was a problem hiding this comment.
All three operators (&, |, ^) can be written much simpler. Core algorithm logic looks like this:
- newBitsNumber = max(BitsNumber, rhs.BitsNumber)
- Perform logic operation on values
- Trim returned BitMask to size == position of the leftmost bit set to 1.
We can discuss this further via pm.
| bool Reset(); | ||
| bool Toggle(size_t index); | ||
|
|
||
| bool Resize(const int offset = 0); |
There was a problem hiding this comment.
size_t size not const int offset
| } | ||
|
|
||
|
|
||
| bool BitMask::Resize(const int offset) |
There was a problem hiding this comment.
Resize should apply given size, not current size + offset. That way this method is simple
BitsNumber = size; BitList.Resize((size + TYPE_BIT - 1)/TYPE_BIT)
| return index / TYPE_BIT; | ||
| } | ||
|
|
||
| BitMask& BitMask::operator|=(const BitMask& rhs) |
There was a problem hiding this comment.
No need to write this like that, simply write { *this = *this | rhs; }. Repeat this for other operators overloads.
| bool BitMask::operator==(const BitMask rhs) const | ||
| { | ||
| if (BitsNumber != rhs.BitsNumber) | ||
| return false; |
There was a problem hiding this comment.
Not necessarly, if one of the bitmasks is prefixed with a lot of zeroes this would still be true.
It would probably be helpfull to add method called Trim() which resizes BitMast to size == position of the leftmost bit set to 1. This way you can first trim, or get size after potential trim operation, and compare them to each other.
2 participants
BitMask working with different unsigned data types. In Unit Tests BitList Dynarray size is checked for Debugging purposes.