Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PascalsTriangle: two more test cases! #274

Closed
wants to merge 2 commits into from
Closed

PascalsTriangle: two more test cases! #274

wants to merge 2 commits into from

Conversation

niklasad1
Copy link

Hi,
Added two more test cases
/Niklas A

@IanWhitney
Copy link
Contributor

Thanks!

A couple of questions.

  • What edge cases do these tests cover that the existing tests don't?
  • Are these tests only useful for Rust, or should we add them to the canonical test suite that all language tracks follow?

@niklasad1
Copy link
Author

niklasad1 commented Apr 16, 2017

Hmm, they are not testing any edge case what I'm aware but I thought that people can get lucky or hardcode in the answer which is much harder with 21 rows ;)

Use them if you want to but the existing tests may be sufficient.

@petertseng
Copy link
Member

petertseng commented Apr 25, 2017

I have no opinion on whether this track should have these tests, so I'm acknowledging that I'm being weak, evading the question, and passing the buck by deferring to x-common. If the same test cases are added to x-common, the Rust track will take them.

It looks like from exercism/problem-specifications#362 that some tracks test 20 but it was not made standard since once it works for four rows it probably works for subsequent rows.

Perhaps going up to five or more helps us because the fifth row is the first time a value appears that does not have 1 as one of its addends. That can be an argument for adding five (or some higher number) to x-common.

I will be happy to post the same opinion in x-common if someone would make the PR.

@petertseng
Copy link
Member

petertseng commented May 7, 2017

We look forward to seeing these cases in x-common at https://github.com/exercism/x-common/blob/master/exercises/pascals-triangle/canonical-data.json and would like to see it merged there before merging here.

Let's reopen this PR when it is merged in x-common!

@petertseng petertseng closed this May 7, 2017
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.

3 participants