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

all-your-base: add to track #189

Merged
merged 1 commit into from
Dec 2, 2016
Merged

all-your-base: add to track #189

merged 1 commit into from
Dec 2, 2016

Conversation

stkent
Copy link
Contributor

@stkent stkent commented Nov 26, 2016

Canonical data here.

The test both bases are negative was skipped as it is redundant given the structure of this implementation.

The canonical data also poses some questions for implementors, which I answered as follows:

What's the canonical representation of zero?

[0]

What representations of zero are allowed?

[0]

Are leading zeroes allowed?

No.

How should invalid input be handled?

By throwing an exception with an appropriate message.

@stkent stkent changed the title dibs: I will implement all-your-base all-your-base: add to track Nov 27, 2016
@jtigger jtigger self-assigned this Dec 1, 2016
@jtigger
Copy link
Contributor

jtigger commented Dec 1, 2016

Are leading zeroes allowed?
No.

No love for the octal literal?!? :)

Copy link
Contributor

@jtigger jtigger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another quality add, @stkent. Thank you, sir.


public final class BaseConverterTest {

/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for the comment.

@Test
public void testLeadingZeros() {
expectedException.expect(IllegalArgumentException.class);
expectedException.expectMessage("Digits may not contain leading zeros.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😢 ... 😉

public void testSecondBaseIsOne() {
final BaseConverter baseConverter = new BaseConverter(2, new int[]{1, 0, 1, 0, 1, 0});

expectedException.expect(IllegalArgumentException.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this time, I had anchor bias and put my ExpectedException rules at the top of the method (prior to the rule, we had the annotation parameter). But you are putting it immediately before the method that should emit the exception... I ❤️ this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jtigger
Copy link
Contributor

jtigger commented Dec 1, 2016

Note that per exercism/problem-specifications#279, the other base conversion exercises are considered deprecated. Let's keep that in mind with #142 and this exercise.

@stkent
Copy link
Contributor Author

stkent commented Dec 2, 2016

@jtigger rebased!

@jtigger jtigger merged commit 4ef21e1 into exercism:master Dec 2, 2016
@stkent stkent deleted the all-your-base branch December 2, 2016 16:32
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