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

change: Add case for target > coins, but still can't make #535

Merged
merged 1 commit into from
Feb 8, 2017
Merged

change: Add case for target > coins, but still can't make #535

merged 1 commit into from
Feb 8, 2017

Conversation

petertseng
Copy link
Member

The current error cases ask us to ensure that two types of targets are
invalid:

  • targets smaller than the smallest coin
  • negative targets

These are both reasonable.

I would react with chagrin if this led student solutions to assume that
these are the only two cases in which one could fail to meet the target
i.e. they assume that any target > the smallest coin is valid.

This commit proposes adding a case that disproves that - the target is
larger than even the largest coin, but you will not be able to get a
combination that reaches the target.

The current error cases ask us to ensure that two types of targets are
invalid:
* targets smaller than the smallest coin
* negative targets

These are both reasonable.

I would react with chagrin if this led student solutions to assume that
these are the only two cases in which one could fail to meet the target
i.e. they assume that any target > the smallest coin is valid.

This commit proposes adding a case that disproves that - the target is
larger than even the largest coin, but you will not be able to get a
combination that reaches the target.
@stkent
Copy link
Contributor

stkent commented Feb 7, 2017

Makes total sense to me; Java track is poised to add this so I'll hold that exercise until this is 👍'd and merged!

@ErikSchierboom ErikSchierboom merged commit 5472708 into exercism:master Feb 8, 2017
@ErikSchierboom
Copy link
Member

LGTM!

@petertseng petertseng deleted the change branch February 8, 2017 09:51
emcoding pushed a commit that referenced this pull request Nov 19, 2018
* bin/generate: Default display success message.

The old behavior was:

```
$ bin/generate isogram
$
```

Which is not particularly friendly or helpful. Did it even do anything?

The new default behavior is to report success to stdout:
```
$ bin/generate isogram
Generated isogram tests version 3
$
```

The original behavior can be replicated by redirecting stdout to /dev/null
```
$ bin/generate isogram > /dev/null
$
```

The `-v` option still provides additional information.
```
$ bin/generate -v isogram
Incremented tests version to 4
Updated version in example solution to 4
Generated isogram tests version 4
```

* Send all output via the logger.

Use composition rather than inheritance for the Logging Repository
decorator.
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.

3 participants