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

sum-of-multiples: Move to functional solution, add tests #149

Merged
merged 3 commits into from
Nov 24, 2014
Merged

sum-of-multiples: Move to functional solution, add tests #149

merged 3 commits into from
Nov 24, 2014

Conversation

sjakobi
Copy link
Contributor

@sjakobi sjakobi commented Nov 19, 2014

Closes #139, #147.

@sjakobi
Copy link
Contributor Author

sjakobi commented Nov 19, 2014

@Dog, @mmakaay: Because your PRs had merge conflicts they're now included in this PR. Do you think that it's a sensible package as a whole?

@Dog
Copy link
Contributor

Dog commented Nov 20, 2014

My initial concern is I think the example assumes zero is the first multiple in the list. I'll try to play with it later tonight.

@sjakobi
Copy link
Contributor Author

sjakobi commented Nov 20, 2014

My initial concern is I think the example assumes zero is the first multiple in the list.

That should be ok under the assumptions given in the test file (the input factors are non-negative, unique and sorted in ascending order).

@Dog
Copy link
Contributor

Dog commented Nov 20, 2014

Then it looks good to me.

kytrinyx added a commit that referenced this pull request Nov 24, 2014
sum-of-multiples: Move to functional solution, add tests
@kytrinyx kytrinyx merged commit 3f80959 into exercism:master Nov 24, 2014
@kytrinyx
Copy link
Member

Thanks, everyone!

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