5
\$\begingroup\$

I coded stuff for an embedded device where int is 24bits. I wanted to port uint24_t and int24_t to windows, so I made a header file to handle it.

The goal was to have uint24_t and int24_t work as it did when I coded for the embedded device, and for it to be 3 bytes long and unpadded.

My current implementation uses 3 bytes to store a 24bit integer, then converts it to a 32bit integer for math operations. I also handled the special bit-wise not case.

Is there anything I could add or change in the int24_type.h file to improve its preformance?

#include <stdio.h>
#include <stdint.h>
#include "int24_type.h"

int main() { //Example usage
    int24_t a = 3;
    uint24_t b = 5;
    int24_t c = a - b;
    int32_t d = -a + b;
    c.print_int24_t(); // Printed -2 | FF FF FE
    printf("\n%d",d); // Printed 2
    return 0;
}

#ifndef INT24_TYPE_H
#define INT24_TYPE_H

class uint24_t {
    private:
        //Stored across 3 bytes
            uint8_t b0;
            uint8_t b1;
            uint8_t b2;
        //Conversions
        uint24_t format24(uint32_t i) { //32bit to 24bit
            uint24_t z;
            z.b0 = i & 255;
            z.b1 = (i >> 8) & 255;
            z.b2 = (i >> 16) & 255;
            return z;
        }
        uint32_t format32(uint24_t z) { //24bit to 32bit
            return (z.b0) | (z.b1 << 8) | (z.b2 << 16);
        }
    
    public:
        //Constructors
        uint24_t() {}
        uint24_t(uint32_t i) {
            b0 = i & 255;
            b1 = (i >> 8) & 255;
            b2 = (i >> 16) & 255;
        }
        operator uint32_t () {
            return (b0) | (b1 << 8) | (b2 << 16);
        }
        
        //Bitwise NOT
        uint24_t operator~() {
            uint32_t z = (b0) | (b1 << 8) | (b2 << 16);
            return format24(~z);
        }
        
        //Debug
        void print_uint24_t() {
            uint32_t w = (b0) | (b1 << 8) | (b2 << 16);
            printf("\n%d | %02X,%02X,%02X",w,b2,b1,b0); 
        }
};

class int24_t {
    private:
        //Stored across 3 bytes
            uint8_t b0;
            uint8_t b1;
            uint8_t b2;
            
        //Conversions
        int24_t format24(int32_t i) { //32bit to 24bit
            int24_t z;
            z.b0 = (uint32_t)(i) & 255;
            z.b1 = ((uint32_t)(i) >> 8) & 255;
            z.b2 = ((uint32_t)(i) >> 16) & 255;
            return z;
        }
        int32_t format32(int24_t z) { //24bit to 32bit
            int32_t w = (z.b0) | (z.b1 << 8) | (z.b2 << 16);
            if (b2 & 128) { //If bit 23 is set
                w |= 0xFF000000;
            }
            return w;
        }
    
    public:
        //Constructors
        int24_t() {}
        int24_t(int32_t i) {
            b0 = (uint32_t)(i) & 255;
            b1 = ((uint32_t)(i) >> 8) & 255;
            b2 = ((uint32_t)(i) >> 16) & 255;
        }
        operator int32_t () {
            int32_t w = (b0) | (b1 << 8) | (b2 << 16);
            if (b2 & 128) { //If bit 23 is set
                w |= 0xFF000000;
            }
            return w;
        }
        
        //Bitwise Not
        int24_t operator~() {
            int32_t z = (b0) | (b1 << 8) | (b2 << 16);
            return format24(~z);
        }
        
        //Debug
        void print_int24_t() {
            int32_t w = (b0) | (b1 << 8) | (b2 << 16);
            if (b2 & 128) { //If bit 23 is set
                w |= 0xFF000000;
            }
            printf("\n%d | %02X,%02X,%02X",w,b2,b1,b0); 
        }
        void print_uint24_t() {
            uint32_t w = (b0) | (b1 << 8) | (b2 << 16);
            printf("\n%d | %02X,%02X,%02X",w,b2,b1,b0); 
        }
};

#endif /* INT24_TYPE_H */
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Frame challenge: have you considered uint_least24_t with a serialize/deserialize that handles endian-ness properly? Then you can just use native types on both platforms, and exchange data safely. \$\endgroup\$ Commented Jun 22, 2023 at 21:05
  • \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate. \$\endgroup\$ Commented Jun 22, 2023 at 23:12
  • \$\begingroup\$ (I guess using a shift by eight for conversion fails as soon as multiplication or shift to&fro are to be supported.) \$\endgroup\$ Commented Jun 24, 2023 at 8:20

1 Answer 1

Reset to default
1
\$\begingroup\$

Write proper C++

Your code uses lots of C-isms: it #includes C headers files and it uses printf(). Instead, write proper C++ code that avoids falling back to C unnecessarily. For example:

#include <iostream>
#include <cstdint>
#include "int24_type.h"

int main() {
    …
    std::cout << d << '\n';
}

Be aware however that in C++ header files, most things are declared inside the std namespace. That includes std::uint32_t and so on. While often it also causes uint32_t to be declared in the global namespace, you cannot rely on that. So I recommend you write std::uint32_t instead.

Put the #includes in the right place

A header file should #include everything it needs itself. So since your classes print and uses the fixed-width integer types, ensure that int24_type.h has #include <iostream> and #include <cstdint> at the top.

You still want to keep the same #includes in the source file containing main(), as you explicitly use the standard fixed-width types and prints there as well. Don't rely on them already being in int24_type.h; consider that you might want to remove the debug printing functions at some point, in which case you should be able to remove #include <cstdio> from int24_type.h, and it would be nice if that didn't cause other source files to break.

Why implement operator~()?

You are relying heavily on the implicit conversion to uint32_t and int32_t for this type to work. But you get ~ for free using implicit conversions as well, so I would remove your operator~()s.

Avoid code duplication

You do the same thing lots of times in your code. For example, converting from 24-bit to 32-bit is done in lots of functions. But you only need to implement it once, for example in the conversion operator. Then the rest of the code can just make use of that conversion operator:

void print_uint32_t() {
    std::uint32_t w = *this;
    …
}

If you've removed operator~(), then format32() is no longer used, so that can be removed entirely. Even before that it was just doing the same as the conversion operator, so it wasn't necessary to begin with.

Prefer = default instead of {}

When you need to add an explicit constructor or assignment operator that has an empty body, use = default instead of {}.

Consider overloading operator<<() for printing

First of all, because if the implicit conversion, and because the output stream classes like std::cout know how to print std::int32_t and std::uint32_t, then unlike printf() in C, in C++ you can actually directly print your classes to std::cout:

std::cout << uint24_t(42) << '\n'; // It just works™!

However, if you want to change how your 24-bit integers are printed, then you could overload operator<<(std::ostream&) to provide a custom way for printing to any C++ stream.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.