Array like accss to the bits of a byte

So I have this class that a friend of mine wrote and there is one statement that confuses me:

Code:

---


#include <iostream>
using namespace std;

class twiddler;

class bits{
    uint8_t& value;
    int n;
    friend class twiddler;
public:
    bits(uint8_t& value, int n):value(value), n(n){}
    bool operator[](int n){return (value & (1 << n)) != 0;}
    void operator=(bool set_value){
        if(set_value)
            value |= (1 << n);
        else
            value &= ~(1 << n);
    }
};

class twiddler{
    uint8_t value;
public:
    twiddler(uint8_t value):value(value){}
    short operator*(){return (short)value;}
    bits operator[](int n){return bits(value, n);}
};

int main(int argc, char** argv){
    twiddler t(8);
    t[0] = true;
    cout << *t << endl;

}

---

In the public section of the twiddler class, why does the "operator[]" overload both have a return type of bits and returns bits, calling it's constructor. I'm not to sure how the methods of the bits class get called. It's like they are being called indirectly or something.

Just go through the code line by line:

twiddler t(8) calls the twiddler constructor twiddler(uint8_t value) and initializes the private member value to the value passed in the constructor argument. So, the twiddler.value now has a value of 8.

t[0] = true invokes operator[] in the twiddler class, which returns a newly constructed bits object, taking as arguments the twiddler's current value and the argument to the operator[]. The bits object is constructed by calling its constructor bits(uint8_t& value, int n), which sets up the bits value to refer to the given value and initializes the n member to the passed n argument. Then the void operator=(bool set_value) is applied to the returned bits object. If the given bit, previously set to n in the constructor, is supposed to be set to true/on/1, as determined by the argument set_value, the stored value is bitwise OR'd with (1 << n), effectively setting bit n to 1 and similarly when the bit is supposed to be cleared.

That said, the class design sucks.

Quote:

---

That said, the class design sucks.

---

Is it that much different from std::bitset?

Quote:

---

Originally Posted by MWAAAHAAA

That said, the class design sucks.

---

If you aren't/ going to provide a better class -or can't-, shut the hell up and keep that kind of opinion to yourself unless you are going to tell me why it sucks. Anyway I understand the entire class, I was just wondering about that one minor thing.

Quote:

---

Originally Posted by Syscal

If you aren't/ going to provide a better class -or can't-, shut the hell up and keep that kind of opinion to yourself unless you are going to tell me why it sucks.

---

Fair enough.

The following immediately stands out:

a) Why are the bits friends with the twiddler? There is absolutely no need for this in the design you presented. The statement should be removed.

b) From a conceptual point of view, the constructor to bits should not take the argument 'n'. This is simply *wrong*. Ask yourself the question why anyone constructing an instance of a bits class (a bitset if you so will), would specify which bit is supposed to be modified by any future operations performed on the bitset. What you should do in the constructor is simply construct the bitset, either an empty one, or a bitset with a predefined set of values. If you want to make things a little fancier you might want to take a second argument specifying the size of the bitset, i.e. the number of bits in the set. But not in any case a parameter which logically belongs to an operation being performed on the bitset.
Such a design would logically allow operations to be performed on just a single predefined bit in the set, whereas this is definitely not the purpose of the class, which becomes evident immediately upon seeing the workaround implemented by the author, consisting of constructing a new instance of the class (and overwriting the old one) everytime the bitset is accessed. What we want, and what is intended, but not reflected in the class design, is to modify the contents of a given bitset, instead of overwriting it completely each time an operation is performed. This is simply not a good design.

c) And this is related to point b) above, why should the parameter n be stored in the class at all? Is the bit we want to operate on an intrinsic property of a bitset? No, it is an argument to an operation that we wish to perform, hence it should be treated as such. Besides which, you are wasting space for a whole int on storing this parameter, which defeats one of the main purposes of defining a bitset in the first place: saving space! Interestingly, the actual bitset is defined as an 8-bit value (probably padded to 4 bytes though). Again, this is not proper design.

Quote:

---

Is it that much different from std::bitset?

---

Superficially, perhaps not, but what class intended to act as a bitset would be? The bitset must be constructed and initialized, no? We have to access individual bits, no? Given the interface description for the std::bitset however, that particular implementation seems not to suffer from b, c above.

Quote:

---

Anyway I understand the entire class, I was just wondering about that one minor thing.

---

Pedantic note: "wondering about that one minor thing" implies that you do *not* "understand the entire class".

Oh, and just as anyone should respect your right to voice your opinion, so should you. So I will damn well not "shut the hell up" thank you very much.

Quote:

---

Originally Posted by MWAAAHAAA

Fair enough.

The following immediately stands out:

a) Why are the bits friends with the twiddler? There is absolutely no need for this in the design you presented. The statement should be removed.

b) From a conceptual point of view, the constructor to bits should not take the argument 'n'. This is simply *wrong*. Ask yourself the question why anyone constructing an instance of a bits class (a bitset if you so will), would specify which bit is supposed to be modified by any future operations performed on the bitset. What you should do in the constructor is simply construct the bitset, either an empty one, or a bitset with a predefined set of values. If you want to make things a little fancier you might want to take a second argument specifying the size of the bitset, i.e. the number of bits in the set. But not in any case a parameter which logically belongs to an operation being performed on the bitset.
Such a design would logically allow operations to be performed on just a single predefined bit in the set, whereas this is definitely not the purpose of the class, which becomes evident immediately upon seeing the workaround implemented by the author, consisting of constructing a new instance of the class (and overwriting the old one) everytime the bitset is accessed. What we want, and what is intended, but not reflected in the class design, is to modify the contents of a given bitset, instead of overwriting it completely each time an operation is performed. This is simply not a good design.

c) And this is related to point b) above, why should the parameter n be stored in the class at all? Is the bit we want to operate on an intrinsic property of a bitset? No, it is an argument to an operation that we wish to perform, hence it should be treated as such. Besides which, you are wasting space for a whole int on storing this parameter, which defeats one of the main purposes of defining a bitset in the first place: saving space! Interestingly, the actual bitset is defined as an 8-bit value (probably padded to 4 bytes though). Again, this is not proper design.



Superficially, perhaps not, but what class intended to act as a bitset would be? The bitset must be constructed and initialized, no? We have to access individual bits, no? Given the interface description for the std::bitset however, that particular implementation seems not to suffer from b, c above.



Pedantic note: "wondering about that one minor thing" implies that you do *not* "understand the entire class".

Oh, and just as anyone should respect your right to voice your opinion, so should you. So I will damn well not "shut the hell up" thank you very much.

---

You act as if the storage of an extra int is extremely demanding these days. Who ever said this was intended to save space? It sure wasn't me. Seeing as a new instance is created, the value of "n" is also gone. So I don't really think it matters that the value is saved, regardless of future operations or not. Another thing, it's a friend of bits because it calls its constructor. It's not as if I couldn't easily re-write this so that it doesn't create a new instance of bits so that the little cry-baby can have his pacifier.

Also, if you read my comment, you would have noticed I said "keep that kind of opinion to yourself unless you are going to tell me why it sucks.". This implies that your opinion will be valued only IF you tell me why it sucks. Heck, who wants to hear some lame ass answer like "the class design sucks" all by itself?

Quote:

---

Originally Posted by MWAAAHAAA

Fair enough.

The following immediately stands out:

a) Why are the bits friends with the twiddler? There is absolutely no need for this in the design you presented. The statement should be removed.

b) From a conceptual point of view, the constructor to bits should not take the argument 'n'. This is simply *wrong*. Ask yourself the question why anyone constructing an instance of a bits class (a bitset if you so will), would specify which bit is supposed to be modified by any future operations performed on the bitset. What you should do in the constructor is simply construct the bitset, either an empty one, or a bitset with a predefined set of values. If you want to make things a little fancier you might want to take a second argument specifying the size of the bitset, i.e. the number of bits in the set. But not in any case a parameter which logically belongs to an operation being performed on the bitset.
Such a design would logically allow operations to be performed on just a single predefined bit in the set, whereas this is definitely not the purpose of the class, which becomes evident immediately upon seeing the workaround implemented by the author, consisting of constructing a new instance of the class (and overwriting the old one) everytime the bitset is accessed. What we want, and what is intended, but not reflected in the class design, is to modify the contents of a given bitset, instead of overwriting it completely each time an operation is performed. This is simply not a good design.

c) And this is related to point b) above, why should the parameter n be stored in the class at all? Is the bit we want to operate on an intrinsic property of a bitset? No, it is an argument to an operation that we wish to perform, hence it should be treated as such. Besides which, you are wasting space for a whole int on storing this parameter, which defeats one of the main purposes of defining a bitset in the first place: saving space! Interestingly, the actual bitset is defined as an 8-bit value (probably padded to 4 bytes though). Again, this is not proper design.



Superficially, perhaps not, but what class intended to act as a bitset would be? The bitset must be constructed and initialized, no? We have to access individual bits, no? Given the interface description for the std::bitset however, that particular implementation seems not to suffer from b, c above.



Pedantic note: "wondering about that one minor thing" implies that you do *not* "understand the entire class".

Oh, and just as anyone should respect your right to voice your opinion, so should you. So I will damn well not "shut the hell up" thank you very much.

---

Good points mhaaawaaa.

Quote:

---

Originally Posted by Syscal

You act as if the storage of an extra int is extremely demanding these days. Who ever said this was intended to save space? It sure wasn't me. Seeing as a new instance is created, the value of "n" is also gone. So I don't really think it matters that the value is saved, regardless of future operations or not. Another thing, it's a friend of bits because it calls its constructor. It's not as if I couldn't easily re-write this so that it doesn't create a new instance of bits so that the little cry-baby can have his pacifier.

Also, if you read my comment, you would have noticed I said "keep that kind of opinion to yourself unless you are going to tell me why it sucks.". This implies that your opinion will be valued only IF you tell me why it sucks. Heck, who wants to hear some lame ass answer like "the class design sucks" all by itself?

---

I suggest you get a clue before arguing, because it sure sounds like you don't have one. The class design does suck,

1. The circular dependency is stupid
2. bits cannot exist without twiddler
3. Your "lack of understanding" comes from the poor design,

Code:

---

    twiddler t(8);
    t[0] = true;

---

Is really:

Code:

---

bits b(8);
b = true;

---

It's so terrible, I don't think it could be worse. Not to mention it gains you nothing...

Quote:

---

Originally Posted by zacs7

I suggest you get a clue before arguing, because it sure sounds like you don't have one. The class design does suck,

1. The circular dependency is stupid
2. bits cannot exist without twiddler
3. Your "lack of understanding" comes from the poor design,

Code:

---

    twiddler t(8);
    t[0] = true;

---

Is really:

Code:

---

bits b(8);
b = true;

---

It's so terrible, I don't think it could be worse. Not to mention it gains you nothing...

---

Yeah well, I'm not a programming demigod. I just have never seen a method return a constructor. I've never even written software for anyone other than myself. Are you suggesting that there is no need for the twiddler class at all?

The only reason I had that negative vibe is Mwuahahaha. He seems to always have some wise-ass remark to say about everything. So we can fight fire with fire. Right?

I still don't get what is so bad about it, as it looks pretty much the same to the mechanism used in std::bitset and std::bitset::reference.

Code:

---

template <size_t>
class bitset
{
...
      /**
      *  This encapsulates the concept of a single bit.  An instance of this
      *  class is a proxy for an actual bit; this way the individual bit
      *  operations are done as faster word-size bitwise instructions.
      *
      *  Most users will never need to use this class directly; conversions
      *  to and from bool are automatic and should be transparent.  Overloaded
      *  operators help to preserve the illusion.
      *
      *  (On a typical system, this "bit %reference" is 64 times the size of
      *  an actual bit.  Ha.)
      */
      class reference
      {
        friend class bitset;

        _WordT *_M_wp;
        size_t _M_bpos;
       
        // left undefined
        reference();
       
      public:
        reference(bitset& __b, size_t __pos)
        {
          _M_wp = &__b._M_getword(__pos);
          _M_bpos = _Base::_S_whichbit(__pos);
        }

        ~reference()
        { }

        // For b[i] = __x;
        reference&
        operator=(bool __x)
        {
          if (__x)
            *_M_wp |= _Base::_S_maskbit(_M_bpos);
          else
            *_M_wp &= ~_Base::_S_maskbit(_M_bpos);
          return *this;
        }

        // For b[i] = b[__j];
        reference&
        operator=(const reference& __j)
        {
          if ((*(__j._M_wp) & _Base::_S_maskbit(__j._M_bpos)))
            *_M_wp |= _Base::_S_maskbit(_M_bpos);
          else
            *_M_wp &= ~_Base::_S_maskbit(_M_bpos);
          return *this;
        }

        // Flips the bit
        bool
        operator~() const
        { return (*(_M_wp) & _Base::_S_maskbit(_M_bpos)) == 0; }

        // For __x = b[i];
        operator bool() const
        { return (*(_M_wp) & _Base::_S_maskbit(_M_bpos)) != 0; }

        // For b[i].flip();
        reference&
        flip()
        {
          *_M_wp ^= _Base::_S_maskbit(_M_bpos);
          return *this;
        }
      };
...
    reference
      operator[](size_t __position)
      { return reference(*this,__position); }

      bool
      operator[](size_t __position) const
      { return _Unchecked_test(__position); }
};

---

The comments in the GCC implementation should answer some questions.

Quote:

---

1. The circular dependency is stupid

---

Yeah, in this case bits does not have to be a friend of twiddler. But may-be OP is just presenting a stripped-down version. Note that there is mutual friendship in GCC's implementation.

Quote:

---

2. bits cannot exist without twiddler

---

Nor is it supposed to.

Also see GCC's comment: "Most users will never need to use this class directly..."

Quote:

---

3. Your "lack of understanding" comes from the poor design,

Code:

---

    twiddler t(8);
    t[0] = true;

---

Is really:

Code:

---

bits b(8);
b = true;

---

It's so terrible, I don't think it could be worse. Not to mention it gains you nothing...

---

Blame C++ for not providing a non-ugly way of referencing individual bits.

Quote:

---

Originally Posted by anon

I still don't get what is so bad about it, as it looks pretty much the same to the mechanism used in std::bitset and std::bitset::reference.

---

They may look pretty much the same, but conceptually there is an important difference.

The std::bitset employs the concept of a std::bitset::reference in order to reference the individual bits. That is to say that the std::bitset acts as a container of sorts for a collection of std::bitset::reference's. These two classes have distinct purposes and this is reflected in their implementations. E.g., one can index a std::bitset using an operator[], as one logically should be able to, whereas this is not possible on a std::bitset::reference.

In the code posted by the OP there is no such distinction between the twiddler and bits classes. In fact, both classes appear to attempt to provide the same kind of functionality, but in a contrived effort to maintain some sort of distinction in the underlying purpose of the classes, the result has become a mangled mess.

The naming of the classes only adds to the confusion. A class named twiddler, being a class that performs the action of twiddling, one would expect to operate on an object, in this case the bitset. Whereas a class named bits, would be expected to contain the individual bit objects on which one desires to operate. In an OO design, one would potentially want to introduce methods into the bits class, which perform the desired operations. This makes the twiddler class seem a stripped down, superfluous copy of the bits class, as it is essentially a copy of the bits class member functions without the underlying data structure to operate on. This is exactly how the class is implemented in the code posted by the OP. And this is yet another reason, perhaps the ultimate reason, why the class design appears messy.

I agree that the classes are poorly named, but it is implementing the exact same pattern as a std::bitset.

Quote:

---

The std::bitset employs the concept of a std::bitset::reference in order to reference the individual bits. That is to say that the std::bitset acts as a container of sorts for a collection of std::bitset::reference's.

---

It is not a collection of references. The references are a proxy for bits, just as the poorly named bits class is a proxy for the poorly named twiddler class.

In OP's code, simply replace "twiddler" with "bitset<8>" and forget about the "bits" class which is just a mechanism that makes the latter work.

In response to the edit you applied while I was preparing my previous post:

Quote:

---

Originally Posted by anon

Blame C++ for not providing a non-ugly way of referencing individual bits.

---

This has been solved in the STL by providing the std::bitset:reference. Separating the referencing entity from the bitset itself leads to a much cleaner design: assigning a boolean value to a bit reference makes sense, assigning a boolean value to a bitset itself however, does not.

Quote:

---

Originally Posted by anon

It is not a collection of references. The references are a proxy for bits, just as the poorly named bits class is a proxy for the poorly named twiddler class.

---

Which is the reason why I referred to it as a container "of sorts". While it is not shown in the code snippet that you posted, I would imagine that the actual bits are simply stored in an int or something similar. Proxy is probably the technically correct term to use for the std::bitset::reference class.

Quote:

---

In OP's code, simply replace "twiddler" with "bitset<8>" and forget about the "bits" class which is just a mechanism that makes the latter work

---

Forgetting about the bits class merely underlines that the design as it is now is simply broken. To use your own words, the bits class is merely designed to make the twiddler work. Isn't the correct terminology in this case a "kludge"?

Quote:

---

Forgetting about the bits class merely underlines that the design as it is now is simply broken. To use your own words, the bits class is merely designed to make the twiddler work. Isn't the correct terminology in this case a "kludge"?

---

I'm sorry but I don't get it. Why is bitset::reference not a kludge and this here is? Because it is not nested? I don't like nesting classes either, though I would put bits in a namespace indicating that this is an implementation detail.

The title is "Array like access to the bits of a byte". This is exactly what std::bitset provides and what OP's code reinvents. The class is unnecessary because it already exists in the standard library, but what makes it so ugly, apart from the choice of class names?

-----------

Edit:

MWAAAHAAA, it seems to me that your confusion comes from thinking that bits is meant to be the bitset equivalent, whereas it should be clear from the user code that twiddler is.

Here's a cleaned-up version, does it still suck?

Code:

---

#include <iostream>
using namespace std;

//class twiddler; //forward declaration not needed

namespace detail { //this class is but an implementation detail
class bit_reference /*bits*/{
    uint8_t& value;
    int n;
    //friend class twiddler; //not needed
public:
    bit_reference/*bits*/(uint8_t& value, int n):value(value), n(n){}
    //bool operator[](int n){return (value & (1 << n)) != 0;}  //does not need this method
    void operator=(bool set_value){
        if(set_value)
            value |= (1 << n);
        else
            value &= ~(1 << n);
    }
};
} //detail

class twiddler{
    uint8_t value;
public:
    twiddler(uint8_t value):value(value){}
    //short operator*(){return (short)value;} //better reserve unary * for pointer-like classes
  short to_short() const { return short(value); }
    detail::bit_reference /*bits*/ operator[](int n){return detail::bit_reference/*bits*/(value, n);}
};

int main(int argc, char** argv){
    twiddler t(8);
    t[0] = true;
    cout << /**t*/t.to_short() << endl;

}

---

OK, operator[] was indeed completely out-of-place in bits.

To OP, as you might gather from the whole thing, you can just use bitset<8>, which provides array-like access to bits (using the same idea as your code), and lots of other functionality in addition.

However, sizeof(bitset<8>) may well be 4 bytes - I guess it is a quality of implementation issue whether they have bothered to provide specializations for smaller sizes. Personally I think a bitset becomes more handy if you need bitsets larger than native integer types.