Test-Cases of `rational-numbers` exercise satisfied by dummy-implementation
rainij opened this issue · 0 comments
The test-cases of rational-numbers
are almost all of the following form:
const expected = new Rational(7, 6)
expect(new Rational(1, 2).add(new Rational(2, 3))).toEqual(expected)
The problem with this is, that if new Rational(x, y)
just provides an empty object {}
(including some dummy methods which do nothing) the test passes just fine. So (all) these tests do not really check if one correctly implements the operations on rational numbers! The only real requirement (in terms of the tests) is that the methods are just there, throw no errors upon calling them and return an object which is equally empty as our dummy-rational-numbers (in terms of data-properties).
The only test-cases which can't be fooled by such a dummy-implementation are the expreal
tests - but please see below for how to "fix" this. Please also read my additional note on expreal
at the end.
My dummy solution which passes all tests
As of now the following "solution" could be pasted into the website and it would pass all tests. The key-insight is that hard-private properties can't be observed by jest. So for toEqual
our rational numbers just look like empty objects (with some methods but no data-properties).
export class Rational {
// Note: These hard-private variables are invisible to jest:
#a: number; // nominator
#b: number; // denominator
constructor(nom: number = 0, den: number = 1) {
this.#a = nom;
this.#b = den;
}
add(other: Rational) {
// I don't feel like I want to implement this ;)
return new Rational();
}
sub(other: Rational) {
return new Rational();
}
mul(other: Rational) {
return new Rational();
}
div(other: Rational) {
return new Rational();
}
abs() {
return new Rational();
}
exprational(n: number): Rational {
return new Rational();
}
expreal(x: number) {
// OK here I really have to do something:
return Math.pow(Math.pow(x, 1 / this.#b), this.#a);
}
reduce() {
return new Rational();
}
}
Possible Fixes
- Require to implement an equality method and test if it works.
- or even simpler: require getters for the nominator and denominator.
For the expreal
issue (see below)
- Relax the test-cases or remove the method entirely.
A note on expreal
This is some extra issue mostly (but not entirely) unrelated to the TypeScript Track in particular. It is more like a general problem with the requirement to add this method. It reflects only my opinion. I am fine if somebody else says that this is not such a big issue.
In my opinion the method expreal
does not fit cleanly into the exercise from a didactic point of view. Everything else in this exercise lives in the realm of rational numbers (although it might be preferable to use bigint
instead of number
to represent the nominator and the denominator but I suspect the exercise is just older then the bigint-proposal reaching typescript). This method is the only one dealing with real numbers. Implementing it is actually rather trivial, however due to floating point issues (and rather strict test cases) one has to fine-tune the solution to satisfy the tests. For example in one test one has to satisfy 8**(4/3)==16
, but please not that usual floating-point rules lead to 8 ** (4/3) === 15.99...98
(test fails), (8 ** (1/3)) ** 4 == 16
(test succeeds). Some students did not bother to search for a reasonable formula which still satisfies the tests, they just reduced the precision of their solution and then it magically worked with the tests - which is probably not intended.