HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Easy Bit IO. Very simple interface works up to N == 64

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simplebitinterfaceeasyworksvery

Problem

After reading this question Easy bitset I/O. I wanted to see if there was a simpler way of doing it.

This is how I would define a way of outputting std::bitset to the stream and reading it back in. It works perfectly with current containers.

BitIO.h

// Include guards deliberately not included for simplicity.

#include 
#include 

template
class BitIO
{
    public:
    std::bitset&     data;
    BitIO(std::bitset& data)
        : data(data)
    {}

    friend std::ostream& operator>(std::istream& stream, BitIO& value)
    {
        // As the current output uses a white space separated
        // unsigned long long it is easy to read it back in using the
        // normal `operator>>`
        //
        // Would be just as simple to convert this function to use
        // a fixed width format to mirror the output. 
        unsigned long long tmp;
        if (stream >> tmp)
        {
            value.data  = std::bitset(tmp);
        }
        return stream;
    }
};


main.cpp

#include "BitIO.h"
#include 

int main()
{
    std::vector >    values { std::bitset(1), std::bitset(2), std::bitset(255) };

    std::copy(std::begin(values), std::end(values), std::ostream_iterator>(std::cout));

    std::bitset  data { 129 };
    std::cout (data);

}

Solution

The code is relatively short and well-written, so I didn't find many nits to pick. With that said, here's my review:

Include all needed files

Since std::ostream_iterator is declared in ` that should be among the includes in main:

#include 


Use appropriate type for template parameter

The
std::bitset template is defined as taking std::size_t rather than int, so it would make sense to define the BitIO template similarly:

template
class BitIO
{
    // ...
};


One can, of course, still abuse the interface by passing negative numbers but at least the user of the code is forewarned that they will be interpreted as
std::size_t rather than as signed numbers.

Consider implementing a
move constructor

At the moment, this potential usage cannot compile:

std::cout (std::bitset{ 99 });


It's easily fixed by adding this constructor:

BitIO(std::bitset&& data)
    : data(data)
{}


Maintain parallelism with existing practice

I was surprised to see spaces separating the outputs when none had been specified in the output
ostream_iterator. Although having the space is arguably useful, the choice of separator (if any) should be left up to the user of the template instead of being hardcoded in the operator <<` function. I'd omit the trailing space to allow usage like this:

std::copy(std::begin(values), std::end(values), 
        std::ostream_iterator>(std::cout,":"));

Code Snippets

#include <iterator>
template<std::size_t S>
class BitIO
{
    // ...
};
std::cout << BitIO<16>(std::bitset<16>{ 99 });
BitIO(std::bitset<S>&& data)
    : data(data)
{}
std::copy(std::begin(values), std::end(values), 
        std::ostream_iterator<BitIO<16>>(std::cout,":"));

Context

StackExchange Code Review Q#67350, answer score: 7

Revisions (0)

No revisions yet.