I have a function that takes an unsigned
value and transforms it into a std::vector<std::uint8_t>
. The requirements (and post conditions) are as follows:
- The code must utilize "modern C++" up to C++14
- The focus is on readability/maintainability first, performance second
- The most significant byte must be at index 0.
- The lease significant byte must be at index
size()-1
. - The return type cannot be changed; this is a requirement stemming from other code not shown in this context (larger application requirement). Also avoids a conversion from one container to
std::vector
. - The resulting vector must be as compact as possible. For values that decompose into fewer bytes than
sizeof(unsigned)
, the resulting vector must not contain unnecessary MSB's (that is, zero value bytes). - The code must be endianness-agnostic and portable to various other architectures and platforms.
Here is the current implementation I have:
using BinaryStreamByteVector = std::vector<std::uint8_t>;
BinaryStreamByteVector CreateMultiByteInteger(unsigned value)
{
BinaryStreamByteVector value_bytes;
while (value != 0)
{
std::uint8_t byte = value & 0xFF;
value_bytes.insert(value_bytes.begin(), byte);
value >>= 8;
}
return value_bytes;
}
I can't think of a better implementation for CreateMultiByteInteger
, so I'm hoping for other ideas that meet the requirements above.
1 Answer 1
Observations
Your code is beautiful (proper naming conventions followed, proper whitespace usage, very readable. I particularly like the type-aliasing, which helps make the whole thing that much more readable than a typedef
or #define
). I cannot personally think of any more ways to make your code more readable, however, I do have 1 suggestion.
The algorithm you currently follow is standard and ideal (you even have proper masking of higher-order bits); and in my honest opinion any other algorithm would be tantamount to obfuscation; but if you assume that the code will only ever run on a particular architecture, you could probably unroll the loop.
Suggestion
Make your function parameter
const
. Although C++ is call-by-value, making it explicitlyconst
and mutating a local copy of the variable is never a bad idea, and it does make your function contract that much clearer.On the performance side, since on any reasonable computer
unsigned
will have maximally 16 bytes (64 bits of 8 bytes is the case for all common architectures, except for probably some microcontrollers which will have less, even as less as 1 byte) (note that the upcoming RISCV architecture does have a native 16-byte (128-bit) integral data type, but the 64-bit one is the default), you can easily make this function a template, such that the computation of the byte vector can be done at compile time without much overhead and incur no runtime overhead.You can probably even pre-set the capacity of your
std::vector
utilisingsizeof(unsigned)
.Although I'm not sure if this suggestion will work in the constraints of your application.
-
1\$\begingroup\$ Ironic that you say here "since on any reasonable computer unsigned will have maximally 4 bytes", yet just replied to me with a comment that says: "It's always better to be independent of as much stuff as possible". While portability has its virtues, I don't agree that it is "always" better to write portable code, and clearly you don't either. :-) \$\endgroup\$Cody Gray– Cody Gray2017年01月11日 16:46:03 +00:00Commented Jan 11, 2017 at 16:46
-
\$\begingroup\$ Eh... that was a typo. I meant 8 bytes. I'll fix it right now. \$\endgroup\$Tamoghna Chowdhury– Tamoghna Chowdhury2017年01月11日 17:00:05 +00:00Commented Jan 11, 2017 at 17:00
-
\$\begingroup\$ @CodyGray Fixed now :p. \$\endgroup\$Tamoghna Chowdhury– Tamoghna Chowdhury2017年01月11日 17:07:30 +00:00Commented Jan 11, 2017 at 17:07
const
array or aconst std:: array
(for retaining size information), which varies the size of the array returned using thesizeof
operator onunsigned
. That would be absolutely performant, I guess. The algorithm is perfect. You even include the masking of the higher-order bits. Unfortunately, you have a restriction on the return type, so this wouldn't help. However, maybe the idea of making it a template function will still help? \$\endgroup\$