In an effort to become more familiar with Kotlin, I've decided to implement a class that represents rational numbers, ie. numbers that can be expressed as one integer divided by another. Numbers represented this way have the advantage that they can be manipulated without any loss of precision (unlike doubles and floats which are imprecise). I've found that rationals are useful, in particular with my work on LastCalc, a web-based calculator/programming language implemented in Java.
It’s still a work in progress, but since I’m new to Kotlin I wanted to get some early feedback in case I’m doing anything dumb - the important class can be found here:
https://github.com/sanity/kotlin-rational/blob/master/src/main/java/us/locut/Rational.kt
As you can see, there is much to be done, including comments and unit tests. It certainly isn’t professional quality yet, not even close. With this in mind I’ve been looking for examples of how to use kunit, but haven’t had much luck. Ultimately my intention is to release this under a sufficiently liberal open source license that it could be integrated into the Kotlin standard library, assuming that it is considered sufficiently useful.
For now I’d be interested in feedback, and am also happy to accept patches. In particular I’m interested in constructive criticism of how Rationals are currently constructed. I’ve made the constructor private so that I can enforce “simplified” being true only if the rational is in its simplest form.
Hi,
I have only minor comments/questions about Kotlin:
- Why do you specify val for function parameters?
- toString() could enjoy string templates: “($nominator/$denominator)”
Your implementation of factory methods is correct.
I noticed some other things, in case you are interested:
- It’s “numerator”.
- Do you really need the simplified flag at all?
- Implementation of equals() does not fulfill the contract: it must be symmetric (see Bloch’s Effective Java), so you can only be equal to a Rational.
Hello,
I would like to add some more comments:
- “;” are optional in kotlin and in general it’s recomended to omit them
- a short form for function declaration can give you less code without losing functionality
class Rational(nominator: Int, denominator: Int = 1) {
public val numerator: Int
public val denominator: Int
{
require(denominator != 0)
val gcd = gcf(Math.abs(nominator), Math.abs(denominator))
val sign = if ((nominator < 0) == (denominator < 0)) 1 else -1
this.numerator = Math.abs(nominator / gcd) * sign
this.denominator = Math.abs(denominator / gcd)
}
…
}
- don’t forget about unary operators:
``
public fun Rational.minus(): Rational = Rational(-nominator, denominator)
public fun Rational.plus(): Rational = this
- in order to use rational numbers as right operand with standard types some extension functions should be added:
public fun Int.plus(r: Rational) : Rational = r + this
- Be carefull with converting double and other numbers to Rational through string representation. Lots of double values can't be converted to Rational. Consider "Infinite" or 1e-100 as example.
Thank you for the feedback Andrey, I've implemented your suggestions.
I use “val” for function parameters to indicate that they are final, in the hope that this permits compiler optimizations. Is “val” assumed if not specified? I can’t find this in the docs.
The “simplified” flag is an optimization, to prevent re-application of gcf unnescessarily. Do you think that gcf is fast enough that this optimization is unnecessary?
Thanks, I've made the function definitions more concise per your suggestion, and removed all of the ;s.
In this example:
``
class Rational(nominator: Int, denominator: Int = 1) {
public val numerator: Int
public val denominator: Int
Why do you define the fields separately, why not just:
``
class Rational(val nominator: Int, val denominator: Int = 1) {
I’ll think a little more about applying gcf in the constructors to guarantee that all Rationals are stored in their simplest form. You might be right that this is preferable.
"val" is assumed by default.
The recursive implementation of gcf (that should be “gcd”, in fact) is not very fast, but a proper implementation would be fast enough, and wouldn’t be called too often, I believe.