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 multiple issue #360

Closed
LukeSkyw opened this issue Oct 8, 2017 · 6 comments
Closed

Sum of multiple issue #360

LukeSkyw opened this issue Oct 8, 2017 · 6 comments

Comments

@LukeSkyw
Copy link

LukeSkyw commented Oct 8, 2017

The sum of multiples exercise says:

Given a number, find the sum of the multiples of a given set of numbers, up to but not including that number.

One of the test is:

#[test]
fn multiples_seven() {
    assert_eq!(30, sum_of_multiples(15, &vec![4, 6]))
}

However:

4 x 1 =>  4
6 x 1 =>  6
4 x 2 =>  8
6 x 2 => 12
4 x 3 => 12
        ---
         42

But the test says 30, like it's ignoring 4 x 3 (the second 12).

@jgilray
Copy link
Contributor

jgilray commented Oct 9, 2017

Perhaps the intention is for sum_of_multiples to give the sum of unique multiples?

@petertseng
Copy link
Member

Only unique multiples are considered; 12 should only be considered once. This is the intent of the example in exercism/problem-specifications#157. If it should be that the description was not clear about it, or can be made clearer, then it will be time to suggest an improvement to https://github.com/exercism/problem-specifications/blob/master/exercises/sum-of-multiples/description.md.

@hekrause
Copy link
Contributor

@petertseng @LukeSkyw
I made a change in rust/problem-specifications and created a pull request.

@andrii-rubtsov
Copy link

andrii-rubtsov commented Nov 1, 2017

I agree that the task assignment formulation is somewhat vague and can probably be improved, but to me, there no such thing as unique multiples, as these multiples are just some of natural numbers: 1,2,3,4....
So saying that they must be unique is the same as saying "natural numbers must be unique".

I think the root of evil is that some folks treat these multiples are independent numbers for each factor, whereas they should not.

@hekrause
Copy link
Contributor

hekrause commented Nov 1, 2017

So the question is what to do?
I think in the end the addition is helping to understand the exercise better. People may think twice when they read the description. So even if it is still clear without the "unique" word, it will become clear to everybody with it.

@coriolinus
Copy link
Member

@hekrause Now that the upstream documentation is updated, would you like to create a PR to update the Rust track for this exercise? The actual work involved should be pretty much configlet generate . --only sum-of-multiples.

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

No branches or pull requests

6 participants