Implement Pascals Triangle #201
Conversation
Official tests have not been merged yet, but I'm mostly following them here. exercism/problem-specifications#362 Example code is naive. Just enough to pass the tests. A real example solution will come later.
There may be a better way to handle the numeric casting I have to do in my row function, but maybe not.
IanWhitney
force-pushed
the
implement_pascals_triangle
branch
from
8b88cba to
f89dc1e
Compare
- `if let` so I can avoid the unwrap. - `extend` to avoid the double `push`
|
Ok. I'm happy with the implementation, so it's time to hear about all the dumb mistakes I made in it. As for position, I think this can go fairly early. The weirdest part of it is returning a Vec of Vecs, and that's not that weird. The meat of this problem is just math, all of which I cribbed from Wikipedia. Other implementations of this I've seen use the values from the previous row to calculate the current row, which means handling out-of-bounds vec indexes. We'll want to take that into account when determining position. |
|
Oh, and all of the number casting. That is kinda weird, if you're used to a language that is more forgiving with numeric types. |
|
API question: or or |
|
|
||
| for p in 1..(number + 1) { | ||
| if let Some(last) = r.pop() { | ||
| let next = last as f32 * ((number as f32 + 1f32 - p as f32) / p as f32); |
There was a problem hiding this comment.
Since you thought all the numeric casting was weird, perhaps you would like to remove it, if you were able? After all, the entries in Pascal's triangle are all integers, and the formula for doing this calculation mentions no rounding of any sort, so that implies it should be possible without resorting to floating point numbers.
then someone may rightfully complain "But written as-is, we do have to cast to floats, because (number + 1 - p) / p may be fractional!"
True, but a * (b / c) is equal to (a * b) / c.
There was a problem hiding this comment.
Someone may then rightfully say "(a * b) / c leaves us more susceptible to integer overflow" though, which I can't deny! Even using u64 will only stave that off for so long. My only defence is "well at least numbers that lrage don't get tested in this exercise"
There was a problem hiding this comment.
I am missing some math concept here.
I had to introduce the floats so that it would properly determine the 3rd element in row 2. With ints, the formula looked like
let next = 2 * ((2 + 1 - 2) / 2);or
let next = 2 * (1 / 2);With ints, 1/2 returns 0. So my 2nd row became [1,2,0]
What am I missing here?
There was a problem hiding this comment.
let next = (2 * 1) / 2 is 1.
Math reasons: exercism#201 (review)
|
I don't think I would mind any of the three proposed APIs. I would place no later than Roman numerals. Probably no earlier than, say... Hamming. |
| for p in 1..(number + 1) { | ||
| if let Some(last) = r.pop() { | ||
| let next = (last * (number + 1 - p)) / p; | ||
| r.extend(&[last, next]) |
There was a problem hiding this comment.
you could use r.last() so that you can just push(next) instead of having to pop the last element and put it back on with extend.
There was a problem hiding this comment.
In my testing, last returns a borrow, so I can't mutate the vector inside the block.
Example: https://play.rust-lang.org/?gist=4420b3f3b8fa09fbeb78b8eb2b3ca99c&version=stable&backtrace=0
There was a problem hiding this comment.
Hmm. I wonder. I thought that was supposed to work, and then it didn't. I quickly tested and found that with the addition of a strategically-placed &, it does work:
if let Some(&y) = x.last() {
x.push(y + 5);
}https://play.rust-lang.org/?gist=a3ace6f29c85b05e9f5027d31d0b4072&version=stable&backtrace=0
I may need a moment to try to puzzle out why one works and not the other. If someone has assistance to offer in this regard, I sure would appreciate it.
There was a problem hiding this comment.
Though obviously we need not let that hold up this PR.
There was a problem hiding this comment.
Just me mis-reading the docs. last() returns Option<&T>, not Option<T>.
IanWhitney
changed the title
Following the official tests
We can't test negative or no rows because the function declares u32 in its signature.