Skip to content

hamming: Change return to Maybe and match json #199

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

Merged
merged 1 commit into from
Jul 13, 2016
Merged

hamming: Change return to Maybe and match json #199

merged 1 commit into from
Jul 13, 2016

Conversation

rbasso
Copy link
Contributor

@rbasso rbasso commented Jul 12, 2016

  • Change return type of distance to a Maybe.
  • Rewrite tests to match exercism/x-common/hamming.hs.

@rbasso rbasso changed the title hamming: Change return to Maybe and match jason hamming: Change return to Maybe and match json Jul 12, 2016
- Change return type of `distance` to a *Maybe*.
- Rewrite tests to match `exercism/x-common/hamming.hs`.
@petertseng
Copy link
Member

👍

@rbasso
Copy link
Contributor Author

rbasso commented Jul 13, 2016

I think the example.hs is kind of ugly. Any idea to improve it?

@petertseng
Copy link
Member

What would be closer to your ideal?

There is the option of checking the length beforehand instead of during, with something like:

distance a b = if length a == length b then Just . sum . map fromEnum $ zipWith (/=) a b else Nothing

But this is pretty much just a little extra stuff slapped onto the previous solution. And this only work with distance :: String -> String -> Maybe Int rather than distance :: Integral a => String -> String -> Maybe a because fromEnum is a -> Int

(I also sometimes wonder whether it traverses a and b one more time than is necessary - one to find the length, then once more to compare them, but I don't know if everyone thinks about such things)

I may think of other ideas later, given some criteria

@petertseng
Copy link
Member

You could move the if x /= y into a guard if you so desire.

@rbasso
Copy link
Contributor Author

rbasso commented Jul 13, 2016

Thanks, @petertseng!

I also feel it traverses the lists two times, but I don't know enough to be sure.
I'll merge it the way it is. If you find any solution you think is good, just open the PR and I'll merge it. The same, of course, applies to other exercises. 👍

@rbasso rbasso merged commit 3094540 into exercism:master Jul 13, 2016
@rbasso rbasso deleted the hamming-change-return-to-maybe-match-json branch July 13, 2016 07:53
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.

2 participants