CODEWITHSUNDEEP
c#.net
LCJ
LCJ

Reputation: 22661

Overflow check does not work

I have following code that has a multiplication of a number with 60000 to convert minutes to milliseconds. I have implemented a overflow check as shown below. Still I am getting the following code analysis warring. How to overcome this warning without suppressing it?

Warning: CA2233: Correct the potential overflow in the operation 'sessionExpiryValueInMinutes*60' in 'ApplicationSessionDAL.IsSessionExpired(short)'

Note: TimeSpan.TotalMilliseconds Property is of double datatype

CODE

    public void IsSessionExpired(Int16 sessionExpiryValueInMinutes)
    {

        if (sessionExpiryValueInMinutes > (double.MaxValue) / 60000)
        {
            //Overflow check
            throw new ArgumentOutOfRangeException("sessionExpiryValueInMinutes");
        }
        else
        {
            //int milliSecondsValue  = sessionExpiryValueInMinutes * 60 * 1000;

            DateTime lastAccessTime = new DateTime(2013, 1, 1);
            TimeSpan elapsedTime = (DateTime.Now - lastAccessTime);
            if (elapsedTime.TotalMilliseconds > (sessionExpiryValueInMinutes * 60 * 1000))
            {
                bool isTimeExpired = true;
            }

        }

    }

REFERENCES

  1. Why is FxCop warning about an overflow (CA2233) in this C# code?

Upvotes: 2

Views: 1381

Answers (2)

Alexandre Vinçon
Alexandre Vinçon

Reputation: 954

I am not sure, but I suspect that it means that the result of (sessionExpiryValueInMinutes * 60 * 1000) will never hold into an Int16.

I see another issue with your code: I'm almost certain that sessionExpiryValueInMinutes, being an Int16, will never ever be greater than (double.MaxValue)/60000.

Upvotes: 0

Jalayn
Jalayn

Reputation: 9284

You may wrap your computation in a checked block. That way, the program will explicitly throw a System.OverflowException which you can here catch to do what you want. And since you want to throw an exception anyway, in your particular case, you don't need to do anything else.

Example:

checked 
{
    if (elapsedTime.TotalMilliseconds > (sessionExpiryValueInMinutes * 60 * 1000))
    {
        bool isTimeExpired = true;
    }
}

And @Oded is right, FxCop cannot be always that smart.

Upvotes: 5

Related Questions