Trying to prevent overflow when adding two longs

Question

I am trying to prevent overflow in two situations, one is when multipling a long with a float, and the other is when adding two longs.

Like:

public static long multiplyLong(long value, float multiplier)
{
    if (value * multiplier > long.MaxValue)
    {
        Debug.Log("greater then max value");
        return value = long.MaxValue;
    }
    else
    {
        Debug.Log("not greater then max value");
        return (long) (value * multiplier);
    }
}

public static long addLong(long value, long adder)
{
    if(value + adder > long.MaxValue)
    {
        Debug.Log("greater then max value");
        return value = long.MaxValue;
    }
    else
    {
        Debug.Log("not greater then max value");
        return value + adder;
    }
}

"multiplyLong" works perfectly, while "addLong" doesn't work like it should. When using addLong, and there will be a overflow it doesn't detect the overflow. I think this is weird because the code seems okay.

Can someone help me?

"checked" isn't an option btw.

Answer 1

Like @Bradley said, you are getting an overflow in your check for overflows. But the problem is bigger than that.

Lets look at this line first

if(value + adder > long.MaxValue)

Here, both value and adder are longs, and adding them together will result in a long. But the problem is that longs automatically roll over instead of throwing exceptions, so if your values are larger than the max value for long, you will just get some other long value seemingly at random.

Now it should be clear why you don't want to do this but before we get into what the correct solution is, lets look at your multiplyLong method. Specifically, the following line

if (value * multiplier > long.MaxValue)

In this value is a long, and multiplier is a float, and multiplying them together results in a float, which can easily be checked against a long so your check is going to appear to work.

But what if the value of value * multiplier is greater than long.MaxValue and cast it as a long?

the answer is that you get long.MinValue, which is certainly less than long.MaxValue

There is much more that can be said on this topic but, this should be enough to convince you that what you are doing is not great.

Now, what should you do? I recommend using the decimal type for two reasons. One, it has a much larger range than long (decimal can go up to 79228162514264337593543950335, where long can only go up to 9223372036854775807), the other reason is that it throws explicit exceptions when overflows happen.

For example, here is my rewritten version of your code

public static long multiplyLong(decimal value, decimal multiplier)
{
    try
    {
        return value*multiplier;
    }
    catch (OverflowException e)
    {
        Debug.Log("greater then max value");
        return decimal.MaxValue;
    }

}

public static long addLong(decimal value, decimal adder)
{
    try
    {
        return value+adder;
    }
    catch (OverflowException e)
    {
        Debug.Log("greater then max value");
        return decimal.MaxValue;
    }
}

Answer 2

You can have two possible overflow cases:

1. Two big positive value and adder return non-positive result
2. Two big negative value and adder return non-negative result

So you have to check both conditions for both borders in the form:

  if ((value + adder < value && value + adder < adder) || 
      (value + adder > value && value + adder > adder)) {
    // Overflow
  }

Or as you, probably, want to put it

public static long addLong(long value, long adder) {
  unchecked { // we'll detect overflow manually
    if (value + adder < value && value + adder < adder) {
      Debug.Log("greater then max value");
      return long.MaxValue;
    }
    else if (value + adder > value && value + adder > adder) {
      Debug.Log("less then min value");
      return long.MinValue;
    }
    else {
      Debug.Log("within the [min..max] range");
      return value + adder;
    }
  }
}

Tests:

addLong(2, 3);                   // within the [min..max] range
addLong(long.MaxValue - 3, 5);   // greater then max value
addLong(long.MinValue + 2, -5);  // less then min value

Answer 3

By performing the addition value + adder (which returns a long) you are causing the overflow in your check for overflow.

Either force the result to something that can hold bigger numbers, ie:

if(value + (Decimal)adder > long.MaxValue)

Or catch the Overflow exception instead of trying to check for the overflow ahead of time. The reason your multiplication check works is your result is a float which can hold bigger numbers.

Note that you can cast either operand, the point is that you are getting the Decimal add operator instead of the long one; the same way you force floating point division.