Skip to content

discussion: Should we return Maybe when there is a possibility of invalid input? #115

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
petertseng opened this issue Apr 7, 2016 · 10 comments

Comments

@petertseng
Copy link
Member

petertseng commented Apr 7, 2016

While working on #114 I noticed that the binary tests just return 0 when there is an error.

That's simpler to implement. The disadvantage is that it makes the two cases "valid input whose value is zero" and "invalid input" indistinguishable to someone who can only observe the output values. Would we like to return Nones for invalid inputs? I did get Maybes added to largest-series-product in #89 .

Personally I would argue for it since I think it's a common thing in Haskell and more exposure to it is good, but there may be a possibility that there's too much. Wonder what others think.

To clarify scope, this is not suggesting that every exercise that possibly can test invalid inputs should (for example, "should the meetup exercise test nonexistent months") - that is a separate question that is worth another issue if someone wish to discuss it. The question is simply: For the exercises that already test invalid inputs, should they return Maybe as the result?

If this is done, at the very least all the base conversion exercises will want it (binary, trinary, octal, hexadecimal). I can possibly investigate the other exercises in Haskell to see where else it would make sense to add too.

Survey of existing exercises and how they handle invalid inputs:

  • base-conversion exercises (binary, trinary, octal, hexadecimal) have already been mentioned - they return 0.
  • nucleotide uses error https://github.com/exercism/xhaskell/blob/master/exercises/nucleotide-count/example.hs and the tests check for it via Control.Exception. Seems a big hammer, whereas I'd rather make the possibility of error apparent in the type system if we're going to check for it (it's one of the reasons why I use Haskell anyway)
  • phone number also returns 0000000000, whereas it could return Maybe
  • say, wordy, zipper already return Maybes.
@petertseng petertseng changed the title discussion: Should we return Maybe when there is a possibility of error? discussion: Should we return Maybe when there is a possibility of invalid input? Apr 7, 2016
@rbasso
Copy link
Contributor

rbasso commented Jun 17, 2016

Those exercises fell awkward and force unidiomatic solutions. It's desirable, at least, to avoid promotion of bad coding practices.

We should probably avoid demanding "error encoding" practices and partial functions most of the time.

I'm not against having a few exercises with not-so-idiomatic interfaces, as they force people to learn other features, but I believe that should be the exception, not the rule. If the problem's description doesn't specify a behavior on invalid input, it should return a Maybe.

I understand it's desirable to have exercises that allow a diversity of solutions, but the need - in Haskell - to type-check the solution with the tests forces us to choose an appropriate interface.

So, I agree! 👍

@rbasso
Copy link
Contributor

rbasso commented Jun 18, 2016

If we are considering rewriting exercises, let's first see how this relates to x-common:

  • binary.md says the solution should handle invalid input.
  • trinary.md demands a result of zero on invalid strings.
  • octal.md says to treat invalid input as octal 0, but that could refer to a single Char or the whole String.
  • hexadecimal.md says the solution should handle invalid input.
  • nucleotide-count.md says nothing about invalid input.
  • phone-number.md talks about bad numbers, but does not say clearly what should be returned.
  • All 4 base-conversion problem descriptions specify input as Strings.
  • nucleotide-count says it receives a DNA string.
  • phone-number talks about cleaning up user entered phone numbers.
  • None of them have a JSON file specifying tests.

The way things are now, the four base-conversion exercises look essentially the same. We can correct their return types, but I guess the best thing is to ask in x-common why we have 4 almost identical exercises.

About nucleotide-count, we have to do something about it. Not only it demands a specific error message, but it also fails under lazy evaluation, because the solution may start outputting before evaluating the expression generating the error. Is it like this by design? If it is, we should move it near harder exercises.

About phone-number, I think we could change it.

So I suggest we solve this issue like this:

What do you think, @petertseng ?

Anyone else has anything to say about this? I would appreciate some feedback here. 😄

@petertseng
Copy link
Member Author

I'm surprised to learn that the specification for the base-conversion exercises is so different. To change the specification in x-common requires notifying all implementing tracks of the change; that would be made simpler if they were combined. We'll see what comes of that.

Decide if we change nucleotide-count or move it down the list to avoid confusing beginners.

If you'd like to know why it is this way, we can look at the commit where it was added. noting that there was discussion on this very issue.

Even knowing the rationale, I still slightly favor changing nucleotide-count to use Maybe. To me it doen't seem to right to ask implementors to write a function that uses error when Maybe is available. I could be convinced otherwise if we think the benefits (of showing students how to deal with error?) are worth it.

Change phone-number's return type to a Maybe or an Either.

Looks like nothing stops us from doing this, good.

@rbasso
Copy link
Contributor

rbasso commented Jun 19, 2016

To try increasing visibility and maybe get a little more participation, I think we should open two issues:

  • What do you think about rewriting nucleotide-count to return Maybe or Either?
  • What do you think about rewriting phone-number to return Maybe or Either?

But first, it would be nice to have a more detailed proposal. What would be the exported functions' type?
Edit: Better to open the issues earlier to get more feedback.

@rbasso
Copy link
Contributor

rbasso commented Jun 21, 2016

Considering that exercism/problem-specifications#276 will probably take some time to finish, I don't see any problem in opening a new issue to change the return types of binary, trinary, octal and hexadecimal too.

That's up to you, @petertseng, if you think it's worthy.

@petertseng
Copy link
Member Author

It's altogether possible we could just go ahead and change the four base conversion exercises if the consolidation of all-your-base is still a ways away. Though depending on how things go with exercism/problem-specifications#276, a better tomorrow may not be that far away, in which case we might just favor being lazy and rather focus the efforts on getting all-your-base implemented for this track (and part of that will be to use either Maybe or Either). I think I may like to see how exercism/problem-specifications#276 progresses by the end of the week to see if we can simply prepare for the better tomorrow.

@rbasso
Copy link
Contributor

rbasso commented Jun 23, 2016

You're right! exercism/problem-specifications#276 is going faster than expected.

@rbasso
Copy link
Contributor

rbasso commented Jun 23, 2016

Looks like all-your-base will be merged soon and we will have an almost language-neutral specification.
I think it is really important, especially for Haskell, to have exercises that allow proper use of type classes to signal invalid inputs.

BTW, thanks for everything, @petertseng. 👍

@rbasso
Copy link
Contributor

rbasso commented Jun 30, 2016

Considering that...

...and that the initial demand was...

The question is simply: For the exercises that already test invalid inputs, should they return Maybe as the result?

I think the initial demand was solved and this issue can be closed. Also, we can deal with other exercises that do not check for invalid inputs on their own issues on a case-by-case basis, as we are doing in #166.

Are you satisfied with that solution, @petertseng 👍 ❓

@petertseng
Copy link
Member Author

Seems good to me, and since I can close it, and you seem to also be in favor, I'll go ahead and do it. If anyone has more to add here I'm sure they can speak up here or elsewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants