Skip to content

palindrome-product: Clarify requirements#897

Merged
jpreese merged 3 commits intoexercism:masterfrom
dkinzer:patch-1
Sep 20, 2017
Merged

palindrome-product: Clarify requirements#897
jpreese merged 3 commits intoexercism:masterfrom
dkinzer:patch-1

Conversation

@dkinzer
Copy link
Copy Markdown
Contributor

@dkinzer dkinzer commented Sep 11, 2017

REF exercism/ruby#305
REF exercism/ruby#723

It's not clear from the current examples what the requirements of this exercise are. In fact, a furtive glance is more likely to cause confusion than give clarity.

In my opinion, this is because for the first example the solution lies within the range of the list of factors, giving on a first impression that we are looking for products that lie within this range.

I think that by listing out the entire list of products (which is small in the case of the range of 1..9) we make it more clear that we are considering all the results.

REF exercism/ruby#305

It's not clear from the current examples what the requirements of this exercise are.  In fact, a furtive glance is more likely to cause confusion than give clarity.

In my opinion, this is because for the first example the solution lies within the range of the list of factors, giving on a first impression that we are looking for products that lie within this range.

I think that by listing out the entire list of products (which is small in the case of the range of 1..9) we make it more clear that we are considering all the results.
`[1, 2, 3, 4, 5, 6, 7, 8, 9]`

The smallest palindrome product is `1`. It's factors are `(1, 1)`.
The largest palindrome product is `9`. It's factors are `(1, 9)`, `(3, 3)`, and `(9, 1)`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's should be Its

@Insti
Copy link
Copy Markdown
Contributor

Insti commented Sep 11, 2017

Looks good 👍
Thanks ❤️

@Insti
Copy link
Copy Markdown
Contributor

Insti commented Sep 11, 2017

Once this is merged it would be great if you would create an issue or a PR back in the exercism/ruby repository about updating the readme from the updated common description.

There is some documentation about how to use configlet to generate the readmes:
https://github.com/exercism/docs/blob/master/language-tracks/exercises/anatomy/readmes.md

Edit: I meant to post this as a reply to exercism/ruby#723, updated to make sense in this context. 😊

@Insti Insti changed the title Clarify palindrome product exercise requirements palindrome-product: Clarify requirements Sep 11, 2017
@dkinzer
Copy link
Copy Markdown
Contributor Author

dkinzer commented Sep 11, 2017

Sure, I'll give that a try once this merged.

`[1, 2, 3, 4, 5, 6, 7, 8, 9]`

The smallest palindrome product is `1`. Its factors are `(1, 1)`.
The largest palindrome product is `9`. Its factors are `(1, 9)`, `(3, 3)`, and `(9, 1)`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests only expect (1,9) and (3,3) as factors.
(and always seem to expect factor pairs in (lowest, highest) order.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point... I forgot I had to sort and uniq my results. I'll udpdate.

The tests do not distinguish between `(a, b)` and `(b, c)`.
Copy link
Copy Markdown
Contributor

@Insti Insti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, thanks.

@jpreese jpreese self-requested a review September 20, 2017 01:31
@jpreese jpreese merged commit 568006d into exercism:master Sep 20, 2017
@jpreese
Copy link
Copy Markdown
Contributor

jpreese commented Sep 20, 2017

Thanks @dkinzer 🎉 !

@dkinzer dkinzer deleted the patch-1 branch September 20, 2017 11:44
shaleh pushed a commit to shaleh/exercism-problem-specifications that referenced this pull request Sep 21, 2017
* Clarify palindrome product exercise requirements

REF exercism/ruby#305

It's not clear from the current examples what the requirements of this exercise are.  In fact, a furtive glance is more likely to cause confusion than give clarity.

In my opinion, this is because for the first example the solution lies within the range of the list of factors, giving on a first impression that we are looking for products that lie within this range.

I think that by listing out the entire list of products (which is small in the case of the range of 1..9) we make it more clear that we are considering all the results.

* Fix typo.

* Update palindrome-products factors examples.

The tests do not distinguish between `(a, b)` and `(b, c)`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants