Skip to content

Why have null as input in pascals-triangle exercise #925

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

Closed
ErikSchierboom opened this issue Oct 5, 2017 · 5 comments · Fixed by #926
Closed

Why have null as input in pascals-triangle exercise #925

ErikSchierboom opened this issue Oct 5, 2017 · 5 comments · Fixed by #926

Comments

@ErikSchierboom
Copy link
Member

In the canonical data for the pascals-triangle exercise, the last test case checks for null/missing input: https://github.com/exercism/problem-specifications/blob/master/exercises/pascals-triangle/canonical-data.json#L50 However, to me this does not make sense at all, as you have to specify a number to calculate pascal's triangle. Having no input does not make any sense, unless you want to calculate a default, but the test case for the null input states that passing null means a failure should occur.

My suggestion is to remove this null error case, as it doesn't really add anything (there alreadt is a negative number case that will enforce error handling).

@snahor
Copy link

snahor commented Oct 5, 2017

It doesn't make any sense either to return -1.

@ErikSchierboom
Copy link
Member Author

@snahor Returning -1 is just a general way of saying that an error has occured, see the header:

An expectation of -1 indicates some sort of failure should occur

@snahor
Copy link

snahor commented Oct 5, 2017

I'm aware of it, but I think that shouldn't be explicit. Some exercises are already using {"error": "..."}.

@petertseng
Copy link
Member

I would refer to #902 here, though I'm not convinced a conclusion arose. Nevertheless, I do not think testing null makes a lot of sense.

In most languages that have static type checking, even if they have nullable references, integers are not reference types, so if the function under test is defined to take an integer, null is not a possible input.

For languages without static type checking, null would be a possible input, but I also generally do not see functions that expect to take integers explicitly check for null (or language equivalent); let me know if I'm wrong. So I don't think the tests need to make students write it.

So, removing null seems fine.

Separately, indeed, that "expected": -1 could change to the error object.

@ErikSchierboom
Copy link
Member Author

I've created a PR to fix this issue. I've not updated the expected return value, as that should be a different PR I think.

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 a pull request may close this issue.

3 participants