Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate round on Int and Long #3235

Closed
scabug opened this issue Mar 31, 2010 · 9 comments
Closed

deprecate round on Int and Long #3235

scabug opened this issue Mar 31, 2010 · 9 comments

Comments

@scabug
Copy link

scabug commented Mar 31, 2010

The problem is that math.round(n) (== java.lang.Math.round(n)) is only defined for Float and Double. If called with Int or Long arguments, they are converted to Float/Double, resulting in an loss of precision.
Example:

math.round(123456789)
res17: Int = 123456792
@scabug
Copy link
Author

scabug commented Mar 31, 2010

Imported From: https://issues.scala-lang.org/browse/SI-3235?orig=1
Reporter: @soc

@scabug
Copy link
Author

scabug commented Mar 31, 2010

@soc said:
A way to fix it is adding two additional overloaded methods round() to the math object:

/** This method prevents the implicit type promotion of Long to Double, which would lead to a loss of precision. */
  @deprecated("Rounding Long won't do anything. There is a high chance that calling this method is a bug in your code.")
  def round(x: Long): Long = x
/** This method prevents the implicit type promotion of Int to Float, which would lead to a loss of precision. */
  @deprecated("Rounding Int won't do anything. There is a high chance that calling this method is a bug in your code.")
  def round(x: Int): Int = x

Another possibility, which might be more correct (but I haven't thought deeply about this):

Move math.round(x: Double) to the Double class as an instance method and math.round(x: Float) to the Float class as an instance method.

@scabug
Copy link
Author

scabug commented Mar 31, 2010

@paulp said:
Whatever might be done here, it is certainly not adding overloads to one method. What about the other couple dozen in the Math object? What about every other method in the world which takes a double or float?

Personally I wish we could at least turn off primitives drifting from one type to another, but I already tried for that and martin did not see the value.

@scabug
Copy link
Author

scabug commented Mar 31, 2010

@soc said:
Replying to [comment:2 extempore]:

Whatever might be done here, it is certainly not adding overloads to one method.
What about the other couple dozen in the Math object?
All other methods

  • are already overloaded (abs, max, min, signum)
  • or are defined to return a floating point numbers (trigonometric functions)

But imho round() should either
a) Not be defined for whole numbers => Move the round() methods to the Float/Double class
b) Return the exact same number for whole numbers => Overload them in math

What about every other method in the world which takes a double or float?
Luckily this applies only to functions which implement some rounding function for floating points to whole numbers.

@scabug
Copy link
Author

scabug commented Mar 31, 2010

@soc said:
Ok, after some research I found out that the round method already exists in the RichDouble and RichInt class and that ceil and floor have the same problem.

It seems that the only way to prevent the promotion of whole numbers to Doubles/Floats when using these methods is to overwrite them in Int and Long (option a) does not work).

I can't think of a more appropriate solution at the moment, but maybe you have a better idea?
As far as I understand this can't be fixed with type bounds, because although the numbers behave a bit as if they had a sub-type relationship, there isn't one actually.

(PPS: I wonder if there is any performance difference between math.round and RichDouble's round because of the implicit conversion from Double to RichDouble...)

@scabug
Copy link
Author

scabug commented Apr 7, 2010

@dubochet said:
This is another case where we don't try to improve over Java: package math — and to some degree RichNumber classes — are straightforward wrappers around Java's math library. It may be imperfect, but being coherent with Java has a value, and we don't believe at this point that a better solution is on the table, or likely without big changes to Scala's type system.

@scabug
Copy link
Author

scabug commented Apr 8, 2010

@soc said:
Correction: Long doesn't seem to get promoted to Double, it returns Int.MaxValue/Int.MinValue with

math.round(_x_L) // x being a sufficiently small/large number

But that's a Java feature, too. So I'll just leave it closed.

@scabug
Copy link
Author

scabug commented Apr 9, 2010

@soc said:
Joshua Bloch, when asked about backward compatibility and things he would like to change (Devoxx 2008):

"It would be totally delightful to go through [Java] Puzzlers,
another book that I wrote with Neal Gafter, which contains all
the traps and pitfalls in the language and just excise them -
one by one. Simply remove them.

There are things that were just mistakes, so for example ...
[misspeaks] ... int to float, is a primitive widening conversion
and happens silently, but is lossy if you go from int to float
and back to int.
You often won't get the same int that you started with.

Because, you know, floats, some of the bits are used for the
exponent rather then the mantissa, so you loose precision.
When you go to float and back to int you'll find that you didn't
have the int you started with.

So, you know, it was a mistake, it should corrected, it would
break existing programs. So I do like the idea of essentially
writing a new language which is very similiar to Java which
sort of fixes all these bad things. And if someone's to call it
'Java', that would be great, too. Just so long as traditional
java source code can still be compiled and run against the
latest VMs. [...]

      *-- Joshua Bloch*

Locke would call that "Argument from authority" :-)

@scabug scabug closed this as completed May 18, 2011
@scabug
Copy link
Author

scabug commented Feb 25, 2014

@adriaanm said:
Minimal "fix" in scala/scala#3581

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants