5

I know that unsigned integers are infamous and generally avoided by C++ devs. I have a class with two int member variables that should not contain negative values:

.
.
.
private:
    int m_Y_AxisLen;
    int m_X_AxisLen;
.
.
.

I have designed the logic of the member functions in a way that prevents any input of negative numbers. So I have made sure that those two members won't be assigned negative values.
But this also brings up some warnings when I use PVS-Studio. For example here:

for ( int row = 0; row < getY_AxisLen( ); ++row )
{
    for ( int column = 0; column < getX_AxisLen( ) - 1; ++column )
    {
        if ( m_characterMatrix[ row ][ column ] == getFillCharacter( ) )
        {
            m_characterMatrix[ row ][ column ] = fillCharacter;
        }
    }
}

PVS-Studio blames me for the indices row and column not being of memsize type. It probably means that I should have used std::size_t row and std::size_t column??
But if I had done it that way then it would still complain and say that comparing unsigned integral type with getY_AxisLen( ) (which returns an int) is dangerous.
So this is the reason that I want to rewrite parts of my class to switch to this:

private:
    uint32_t m_Y_AxisLen;
    uint32_t m_X_AxisLen;

I am humbly looking for insights and advice from professionals who have dealt with these kinds of problems before. What would be your approach when it comes to these issues?

digito_evo
  • 3,216
  • 2
  • 14
  • 42
  • 13
    *I know that unsigned integers are infamous and generally avoided by C++ devs* - That's not true. – Evg Dec 21 '21 at 08:10
  • 10
    Why would C++ developers "avoid" unsigned integers? They are generally easier to work with, especially since over- and under-flow doesn't have undefined or implementation defined behavior, works better with bitwise operations, and make much more sense for many things like for example array indexes or sizes. – Some programmer dude Dec 21 '21 at 08:11
  • @Some programmer dude Maybe because they cannot be negative so there is the risk of a wrap-around? Anyway, I have read that C and also C++ are not good at dealing with unsigned values and their use cases should be limited. So you suggest that I better switch to `uint32_t`? – digito_evo Dec 21 '21 at 08:18
  • 1
    @digito_evo `uint32_t` is an unsigned type... – Jabberwocky Dec 21 '21 at 08:18
  • 6
    _"I have read that C and also C++ are not good at dealing with unsigned values"_: where have you read this? It sounds pretty much like nonsense to me. – Jabberwocky Dec 21 '21 at 08:19
  • @Jabberwocky It was in the answer to a question on Quora. It also had mentioned a few valid reasons. – digito_evo Dec 21 '21 at 08:21
  • 1
    @digito_evo Quora and programming questions.... – Jabberwocky Dec 21 '21 at 08:22
  • Some programmers may not good at dealing with unsigned values, not the language itself. – 김선달 Dec 21 '21 at 08:43
  • @digito_evo: `column < getX_AxisLen( ) - 1` is the dangerous part with `unsigned`. when `getX_AxisLen` is `0`. it should be `column + 1 < getX_AxisLen( )`. – Jarod42 Dec 21 '21 at 09:00
  • @Jarod42 Right. In another member function, I have used a condition to prevent the input of any number less than 2 for `m_X_AxisLen`. So even if it's 2, then **2-1 == 1** which means only one iteration. – digito_evo Dec 21 '21 at 09:04
  • 1
    We really want to keep justification close to the point where it's needed. This makes reasoning about the code (much) simpler. So "another member function" doesn't really count. If you can make code more robust right here, I don't see a good reason not to do it even if now it is not needed for formal code correctness. Who knows how this code could be refactored in the future. – Evg Dec 21 '21 at 09:59

1 Answers1

7

A lot of those "you shouldn't use unsigned integers" are just basically scared that you will mix up signed integers and unsigned ones, causing a wrap-around, or to avoid complex integer promotion rules.

But in your code, I see no reason not to use uint32_t and std::size_t, because m_X_AxisLen and m_Y_AxisLen should not contain negative values, and using uint32_t and std::size_t makes a lot more sense here:

So, I suggest changing m_X_AxisLen and m_Y_AxisLen to:

std::size_t m_Y_AxisLen;
std::size_t m_X_AxisLen; // for consistency

Change row and column to

std::size_t row = 0;
// and 
std::size_t column = 0;

Make getX_AxisLen( ) returns an std::size_t

And make the for loop:

for ( int column = 0; column < getX_AxisLen( ) - 1; ++column )

to:

for ( int column = 0; column + 1 < getX_AxisLen( ); ++column )

Because if getX_AxisLen() returns 0, getX_AxisLen( ) - 1 will cause a wrap-around.

Basically, use the thing that makes sense. If a value cannot be negative, use the unsigned type.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
  • Why shouldn't `getX_AxisLen( )` return its associated type (uint32_t)? Wouldn't it be implicitly and safely convertible to `std::size_t`? Cause both are unsigned. – digito_evo Dec 21 '21 at 08:27
  • @digito_evo Do you have a reason to prefer `std::uint32_t` to `std::size_t`? – Evg Dec 21 '21 at 08:28
  • @Evg A few reasons. It's smaller and also it's the data type of `m_X_AxisLen`, so I wonder why return another type? – digito_evo Dec 21 '21 at 08:39
  • @digito_evo Using `std::size_t` to represent sizes is more idiomatic than using `std::uint32_t`. If you don't have a special reason, it might be a good idea to keep `std::uint32_t` in the implementation only and don't expose it to the outside world. – Evg Dec 21 '21 at 08:44
  • @Evg So can I use `std::size_t` for loop counters and make `getX_AxisLen( )` return a `uint32_t` to be compared with the loop counter inside the loop's condition? Changing those two integers to `size_t` means that my class will have a 48-byte alignment meanwhile I want to keep it at 40 bytes. – digito_evo Dec 21 '21 at 08:48
  • @digito_evo I think that you should use just `std::size_t`, to maintain consistency. – justANewb stands with Ukraine Dec 21 '21 at 08:50
  • 1
    @digito_evo You can do it. Note that in C++ it should be `std::uint32_t`. `uint32_t` is not guaranteed to be available in the global namespace. – Evg Dec 21 '21 at 08:51
  • @digito_evo: `column < getX_AxisLen( ) - 1` is the dangerous part with `unsigned`. when `getX_AxisLen` is `0`. it should be `column + 1 < getX_AxisLen( )`. – Jarod42 Dec 21 '21 at 09:00
  • One last question regarding `std::size_t`. So in a 64-bit system, does the *8-byte* `std::size_t` have any performance advantage to the *4-byte* `std::uint32_t`?? – digito_evo Dec 21 '21 at 09:15
  • @digito_evo How many bits `std::uint32_t` have depends on the compiler. It will not affect much. You won't feel any difference between them – justANewb stands with Ukraine Dec 21 '21 at 09:18
  • On mine, it's 4 bytes. I was asking for this since the native size for 64-bit CPUs is 8 bytes. So I wondered if it makes any difference to use the native word size. – digito_evo Dec 21 '21 at 09:23
  • @digito_evo A compiler could use a wider (64-bit) type for an index even you say it's `std::uint32_t` ("as if" rule). The only way to know for sure is to look at the generated assembly code. – Evg Dec 21 '21 at 09:55