patterncppMinor
Bit packing and unpacking
Viewed 0 times
andbitpackingunpacking
Problem
I needed a variadic function to pack and unpack bits into integer types. This is my first attempt:
Usage example:
Does this code contain any bugs, can it be improved, and are variadic templates used appropriately here (I still don't master their syntax)?
template
constexpr T pack(bool b)
{
return b;
}
template
constexpr T pack(bool b, Types... args)
{
return (b (args...);
}
template
void unpack(T packed, bool& b1)
{
b1 = packed & 1;
}
template
void unpack(T packed, bool& b1, Types&... args)
{
b1 = packed & (1 << sizeof...(Types));
unpack(packed, args...);
}Usage example:
int main(void)
{
std::cout (1, 0, 0, 1, 0, 1, 1, 0) (1, 0, 1) (1, 0, 1);
bool b1, b2, b3;
unpack(val, b1, b2, b3);
std::cout << b1 << " " << b2 << " " << b3 << std::endl; // 1 0 1
}Does this code contain any bugs, can it be improved, and are variadic templates used appropriately here (I still don't master their syntax)?
Solution
Missing overflow check
One serious flaw that I see in this code is that the user needs to determine the number of parameters to
One possible solution would be to return an
The other solution would be to retain the current interface but to introduce error notification (by exception or return code).
Unnecessary template parameter
Another thing regarding the
It seems that you don't need the
Naming
The function parameter names could be better.
I assume
One serious flaw that I see in this code is that the user needs to determine the number of parameters to
unpack. This is a no go for a function whose output depends on a runtime input. Even worse there is no check that would tell the user that the actual number overflows the number of bits given to be unpacked into.One possible solution would be to return an
std::vector to fit the whole number.The other solution would be to retain the current interface but to introduce error notification (by exception or return code).
Unnecessary template parameter
Another thing regarding the
unpack function:It seems that you don't need the
Types parameter in the recursion end, so it becomes:template
void unpack(T packed, bool& b1)
{
b1 = packed & 1;
}Naming
The function parameter names could be better.
I assume
b1 is only appropriate in the top level call of the function. I cannot decide on a better name now, but it should indicate that it is the current bit. Likewise, args should be named remainingBits or something like this.Code Snippets
template<typename T>
void unpack(T packed, bool& b1)
{
b1 = packed & 1;
}Context
StackExchange Code Review Q#46303, answer score: 5
Revisions (0)
No revisions yet.