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

binary: Add error handling #171

Merged
merged 2 commits into from
Jun 22, 2015
Merged

binary: Add error handling #171

merged 2 commits into from
Jun 22, 2015

Conversation

kytrinyx
Copy link
Member

This adds some additional test cases to clarify the behavior around
the conversion of valid/invalid binary input.

See exercism/problem-specifications#95

kytrinyx and others added 2 commits June 20, 2015 08:20
This adds some additional test cases to clarify the behavior around
the conversion of valid/invalid binary input.

See exercism/problem-specifications#95
@kotp
Copy link
Member

kotp commented Jun 20, 2015

The example does not adhere to the "do the conversion yourself" but it is a sanity check that everything is working as it should be... trusting the implementers of the language more than a 'self made' solution. The educational value is not in the example.rb file itself, so I think this is OK to do.

@kytrinyx
Copy link
Member Author

The example does not adhere to the "do the conversion yourself" but it is a sanity check that everything is working as it should be... trusting the implementers of the language more than a 'self made' solution.

I'm not sure what you mean. The example does the conversion manually.

@kotp
Copy link
Member

kotp commented Jun 21, 2015

Yes, but it uses a built in method that does this conveniently, rather than writing the implementation explicitly. Using to_s(base) is probably not really the goal, based on the README. But it is definitely good as the definitive example that is expected in the way that it is to work.

@kotp
Copy link
Member

kotp commented Jun 21, 2015

Specifically, my changes to example.rb as:

  def to_decimal
    digits.to_i(2)
  end

@kytrinyx
Copy link
Member Author

The point is to not use to_s(base), though.

@kytrinyx
Copy link
Member Author

Oh, I didn't see your changes. That makes sense. I don't want to use digits.to_i(2) since the whole point of the exercise is to understand binary conversion enough to do it manually.

@kotp
Copy link
Member

kotp commented Jun 21, 2015

Yes, but the nitpickers don't see the example, right? It is a different view point?

@kotp
Copy link
Member

kotp commented Jun 21, 2015

if I do the 'thing to measure against' in the tests, then it sort of exposes this. If I leave it in the more 'hidden' example.rb file, then it does what is intended, cleanly.

@kytrinyx
Copy link
Member Author

Right now the example solution is only used to test the test suite. I guess that means that it's OK to use a solution that would be discouraged on the site, but it seems wrong to me in a way that I'm unable to articulate.

@kotp
Copy link
Member

kotp commented Jun 21, 2015

I have the same little nag as well. I think it is because of the conflicting point of view. It does avoid bugs that a 'homegrown' solution could potentially introduce though.

@kytrinyx
Copy link
Member Author

I see how it could avoid the bugs that a homegrown solution might have, but this is really a trivial problem, and the homegrown solution shouldn't have bugs.

@kotp
Copy link
Member

kotp commented Jun 21, 2015

Then just the style changes then...

@kytrinyx
Copy link
Member Author

Okay :)

kytrinyx added a commit that referenced this pull request Jun 22, 2015
@kytrinyx kytrinyx merged commit ff57124 into master Jun 22, 2015
@kytrinyx kytrinyx deleted the binary-errors branch June 22, 2015 01:57
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