CodeConverter icon indicating copy to clipboard operation
CodeConverter copied to clipboard

VB -> C#: `(int)Math.Round(x)` is used instead of `Convert.ToInt32(x)`

Open TymurGubayev opened this issue 11 months ago • 5 comments

VB.Net input code

Dim i As Integer
Dim d As Decimal
i = d

Erroneous output

int i;
var d = default(decimal);
i = (int)Math.Round(d);

This isn't broken (I don't think so), just very confusing to suddenly see rounding where there previously was none.

Expected output

int i;
var d = default(decimal);
i = Convert.ToInt32(d);

This has the additional benefit of looking the same as the VB.NET code decompiled to C# looks like: SharpLab

Interestingly, explicit operator int(Decimal value) seems to do things slightly differently, so (int)d may not be the same as Convert.ToInt32(d).

Details

  • Product in use: VS extension
  • Version in use: 9.2.5.0
  • Did you see it working in a previous version, which? No
  • Any other relevant information to the issue, or your interest in contributing a fix.

TymurGubayev avatar Mar 06 '24 16:03 TymurGubayev

Both are equivalent.

Convert.ToInt32: https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Convert.cs,11fd4940860023d5

(int): https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Decimal.cs,950

Math.Round: https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Math.cs,85130852558b2bc9

tats-u avatar Apr 10 '24 10:04 tats-u

Although they boil down to equivalent code I would still prefer Convert.ToInt32 if that is what the VB compiler uses. Then it's guaranteed to always be exactly equivalent (for all runtime implementations) and also more clear IMO.

Sympatron avatar Apr 10 '24 11:04 Sympatron

I forgot to say that I just meant that its priority is lower than you think. Both results are fine for me. Math.Round makes it clear for readers that VB's CInt uses the bankers' rounding instead of the truncation. Both have pros and cons.

tats-u avatar Apr 10 '24 12:04 tats-u

Both are equivalent.

Convert.ToInt32: https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Convert.cs,11fd4940860023d5

(int): https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Decimal.cs,950

Math.Round: https://source.dot.net/#System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Math.cs,85130852558b2bc9

Convert.ToInt32(1.5M) == 2
&&
(int)1.5M == 1 &&  decimal.ToInt32(1.5M) == 1
&&
(int)Math.Round(d) == 2

I forgot to say that I just meant that its priority is lower than you think. Both results are fine for me. Math.Round makes it clear for readers that VB's CInt uses the bankers' rounding instead of the truncation. Both have pros and cons.

I would need to check every time what's the default value for Math.Round's MidpointRounding, and it would still surprise me every time.

TymurGubayev avatar Apr 10 '24 14:04 TymurGubayev

I would need to check every time what's the default value for Math.Round's MidpointRounding

It confuses those who mainly use C/C++, Java, and JS, as you think. Some other languages (e.g. Python 3 and Kotlin) treat their round function as bankers' rounding like C# and VB. I assumed those who mainly use C# but do not get used to VB. I think you might know this, but the implementation of the overload with that argument is independent.

tats-u avatar Apr 10 '24 23:04 tats-u