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++;
}

Answer 1

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.