Implemented exercise series

[kedeggel]

No description provided.

[coriolinus]

Why this interface? I'm not categorically rejecting it at this time, but I'd want a solid reason why the tested interface shouldn't be simply pub fn series(digits: &str, len: usize) -> Option<Vec<String>>.

[kedeggel]

I think you‘re right. First I wanted to try another approach where I needed this struct, but then I rejected this and forgot to remove it.

[coriolinus]

In fact, we don't even need the Option; we could have the function just return a 0-length Vec if the consumer requests a series of length greater than the input length, and lose no information.

[kedeggel]

I like my approach with Result because this is one of the few rusty things in this exercise.

[coriolinus]

Using Result to use more Rust features when it's not the natural interface is an antipattern.

There are plenty of exercises for which Option and Result make sense, and are implied by the requirements of the problem. This is not one of them: you know before ever entering the function exactly how many items you expect to get back from it.

[kedeggel]

What would be expected if len = 0? A 0-length Vec?

[coriolinus]

My instinct in that case would actually be to expect vec!["".to_string(); digits.len() + 1].

Reasoning: in all other valid cases, you expect a Vec composed of len-length sequences, which contains digits.len() + 1 - len-length strings. Therefore, this sticks to the pattern.

In general, it's good practice to minimize potential surprise to the user.

[software-artificer]

Just ran into this exact problem and wanted to understand how the decision was made, so ended up here.

I am really surprised by what actually happened, contrary to the intention of "In general, it's good practice to minimize potential surprise to the user.". If this has been a library, and I somehow screwed up my math for length and ended up with zero, I would never expect this function to spit out 6 empty strings - that's completely counter-intuitive 😕

If that's the desired behavior, it should definitely be documented in the exercise itself. Also, literally every other language I checked does raise the exception or return an error.

[coriolinus]

If we use a simpler, function-based interface, there's likely a one-line solution using slice::windows, which makes me question the 4 difficulty here a bit.

If we use the struct-based interface, most of the complexity of the exercise comes from implementing the struct itself, which isn't that interesting.

[kedeggel]

👍

[kedeggel]

I changed difficulty to 1 because there are only the difficulties 1, 4, 7, and 10 in use (according to the last point in https://github.com/exercism/rust/blob/master/README.md#contributing-a-new-exercise).

[coriolinus]

Ok, looks good to me.

I'm going to now leave this for a while to give other reviewers a chance to make their comments, if desired. If there's no other traffic, I intend to merge this on 7 Dec. If on the 8th I still haven't, please feel free to ping me as a reminder.

[petertseng]

I agree that there is no six-length series in a five-character string, and that it is not an error to ask for one.

I think I've been convinced that there should be len + 1 zero-length series. The other leading alternative one would reasonably consider is a single zero-length series. However, just as series("2121", 2) will give you two instances of "21" (in addition to "12", but that is not important), so too does it make sense to include multiple instances of the zero-length series.

There is therefore nothing I would change. If exercism/problem-specifications#1020 is merged as is, we can deal with it then; for example it could suffice to comment that we expect something different for 0 and 6 and leave them as is.

It seems to only make sense to squash these commits. So either the maintainers use Squash and Merge, or the commits be squashed using any other method that is convenient.

[kedeggel]

Squashed