Converting char array to uint16_t array C/C++

Question

I've written the below code to convert and store the data from a string (array of chars) called strinto an array of 16-bit integers called arr16bit

The code works. However, i'd say that there's a better or cleaner way to implement this logic, using less variables etc.

I don't want to use index i to get the modulus % 2, because if using little endian, I have the same algorithm but i starts at the last index of the string and counts down instead of up. Any recommendations are appreciated.

// assuming str had already been initialised before this ..

int strLength        = CalculateStringLength(str);      // function implementation now shown 
uint16_t*  arr16bit  = new uint16_t[ (strLength /2) + 1];  // The only C++ feature used here , so I didn't want to tag it
int indexWrite       = 0;
int counter          = 0;

for(int i = 0; i < strLength; ++i)
{
    arr16bit[indexWrite] <<= 8;
    arr16bit[indexWrite] |= str[i];
    if ( (counter  % 2) != 0)
    {
        indexWrite++;
    }
    counter++;
}

`new uint16_t( (strlen /2) + 1)` allocates memory for *one single* `uint16_t` value, and initializes that single value to `(strlen /2) + 1` — Some programmer dude, Apr 20 '20 at 12:28
You don't have a specific problem but want suggestions on how to improve the code. So question is more suitable for [Code Review SE](https://codereview.stackexchange.com/). Also, this is C++ code and not C code. — kaylum, Apr 20 '20 at 12:29
_"if using little endian, I have the same algorithm but i starts at the last index of the string and counts down instead of up."_ No it doesn't; only the bytes within each pair are swapped — Asteroids With Wings, Apr 20 '20 at 12:34
Why do you have both `counter` and `i` which do exactly the same thing and always hold the same value? And why do you have `indexWrite` which is always exactly half (per integer division) of both of them? — Asteroids With Wings, Apr 20 '20 at 12:35
`uint16_t* arr16bit = new uint16_t( (strlen /2) + 1);` - use `std::vector` — KamilCuk, Apr 20 '20 at 12:36
@AsteroidsWithWings Because if I want to arrange the bytes the other way around in the u16 array, I have another algorithm which counts from strLength -1 backwards — Engineer999, Apr 20 '20 at 12:36
You didn't state that in the question. So the whole problem is under-specified and nobody can help you. — Asteroids With Wings, Apr 20 '20 at 12:39
Also `(strLength/2) + 1` is overallocating one element. It's better to allocate it only if it's needed `(strLength/2) + (strLength%2)` — KamilCuk, Apr 20 '20 at 12:41

Asteroids With Wings · Accepted Answer · 2020-04-20T13:21:59.657

Yes, there are some redundant variables here.

You have both counter and i which do exactly the same thing and always hold the same value. And you have indexWrite which is always exactly half (per integer division) of both of them.

You're also shifting too far (16 bits rather than 8).

const std::size_t strLength = CalculateStringLength(str);
std::vector<uint16_t> arr16bit((strLength/2) + 1);

for (std::size_t i = 0; i < strLength; ++i)
{
    arr16bit[i/2] <<= 8;
    arr16bit[i/2] |= str[i];
}

Though I'd probably do it more like this to avoid N redundant |= operations:

const std::size_t strLength = CalculateStringLength(str);
std::vector<uint16_t> arr16bit((strLength/2) + 1);

for (std::size_t i = 0; i < strLength+1; i += 2)
{
    arr16bit[i/2]      = (str[i] << 8);
    arr16bit[(i/2)+1] |= str[i+1];
}

You may also wish to consider a simple std::copy over the whole dang buffer, if your endianness is right for it.

Thanks for your reply. I realise that I made a mistake and meant <<8 in my question and not <<16. I want to fit each two bytes into one ui16 variable. If reversing the endianess tho, i would start i at strLength -1 and decrement to zero, therefore the above logic would need to be changed a bit — Engineer999, Apr 20 '20 at 12:53
@Engineer999 That's not what endianness is commonly understood to mean. — Asteroids With Wings, Apr 20 '20 at 13:21

Converting char array to uint16_t array C/C++

1 Answers1