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

Template for endianness-free code, data always packed as BIG-Endian

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

Problem

It's been a while since I've been properly grilled about my code.

I had this Idea which should probably never make it into production code but still I couldn't find anything seriously wrong with it at a glance...

All your comments are welcome.

```
#ifndef BITFIELDMEMBER_H
#define BITFIELDMEMBER_H

/**
* This is for creating Endianness-free bit-fields.
* The values are always packed as big-endian.
* When used for arithmetics/comparison, the values are read in the
* proper endianness for the current CPU mode.
*
* Note: The fields can overlap if you want
*/

template
struct BitFieldMember
{
typedef BitFieldMember self_t;
typedef unsigned char uchar;
enum {
lastBit = firstBit + bitSize - 1,
mask = (1 (this);
}
const uchar *selfArray() const
{
return reinterpret_cast(this);
}

/ used to read data from the field /
/ will also work with all the operators that work with integral types /
inline operator unsigned() const
{
const uchar *arr = selfArray();
const uchar *p = arr + firstBit / 8;
int i = 8 - (firstBit & 7);
unsigned ret = 0;
ret |= *p;
while (i > (7 - (lastBit & 7))) & mask);
}

/ used to assign a value into the field /
inline self_t& operator=(unsigned m)
{
uchar *arr = selfArray();
m &= mask;
unsigned wmask = ~(mask >= 8;
wmask >>= 8;
(*(--p) &= wmask) |= m;
i += 8;
}
return *this;
}

inline self_t& operator+=(unsigned m)
{
this = this + m;
return *this;
}

inline self_t& operator-=(unsigned m)
{
this = this - m;
return *this;
}

inline self_t& operator*=(unsigned m)
{
this = this * m;
return *this;
}

inline self_t& operator/=(unsigned m)
{
this = this / m;

Solution

Your code is good, except for two methods : operator unsigned() const and self_t& operator=(unsigned m). They are too complex, and by just looking at them, it is far from clear what they are doing.

First, you can add new values to your enum :

enum {
    lastBit = firstBit + bitSize - 1,
    mask = (1 << bitSize) - 1,
    firstBitIndex = 8 - (firstBit & 7),                         // change to proper name
    nameThisValueProperly = (7 - (lastBit & 7)) & mask,         // change to proper name
};


First modify your operator unsigned() const to this :

inline operator unsigned() const
{
    const uchar *arr = selfArray();
    const uchar *p = arr + firstBit / 8;

    unsigned result = CalculateSomeValue( p );

    return (result >> nameThisValueProperly );
}

// name this method properly
inline unsigned CalculateSomeValue( const uchar *p ) const
{
    int i = firstBitIndex;
    unsigned result = *p;

    while (i < bitSize)
    {
        result <<= 8;
        result |= *(++p);
        i += 8;
    }

    return result;
}


Of course, you need to put proper names both for enum values and method names, so when someone looks into this code says "Aha, that is what is is doing". This way, you can easy increase code clarity.

Also, by making method smaller, you can easily test them.

More about this method, you can read in Clean code by Robert Martin.

You can do something similar for the self_t& operator=(unsigned m). Break it up, until you find it clean.

Code Snippets

enum {
    lastBit = firstBit + bitSize - 1,
    mask = (1 << bitSize) - 1,
    firstBitIndex = 8 - (firstBit & 7),                         // change to proper name
    nameThisValueProperly = (7 - (lastBit & 7)) & mask,         // change to proper name
};
inline operator unsigned() const
{
    const uchar *arr = selfArray();
    const uchar *p = arr + firstBit / 8;

    unsigned result = CalculateSomeValue( p );

    return (result >> nameThisValueProperly );
}

// name this method properly
inline unsigned CalculateSomeValue( const uchar *p ) const
{
    int i = firstBitIndex;
    unsigned result = *p;

    while (i < bitSize)
    {
        result <<= 8;
        result |= *(++p);
        i += 8;
    }

    return result;
}

Context

StackExchange Code Review Q#54342, answer score: 5

Revisions (0)

No revisions yet.