scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

deprecate round on Int and Long #3235

Closed scabug closed 13 years ago

scabug commented 14 years ago

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 commented 14 years ago

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

scabug commented 14 years ago

@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 commented 14 years ago

@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 commented 14 years ago

@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 commented 14 years ago

@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 commented 14 years ago

@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 commented 14 years ago

@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 commented 14 years ago

@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 commented 10 years ago

@adriaanm said: Minimal "fix" in https://github.com/scala/scala/pull/3581