protein-translation: slightly confusing setup

ErikSchierboom commented 5 months ago

I'm not really sure why the protein-translation exercise has an additional parse function that returns a pub struct CodonsInfo<'a>. It feels a bit weird to me. Maybe a better approach would be to model the codons as an enum?

senekor commented 5 months ago

Looks like the exercise was added in this PR: https://github.com/exercism/rust/pull/603.

There's no discussion on this design descision. It seems to me that it gives students the freedom to prepare the input data for efficient lookup (e.g. hash map), which can then be performed many times. (even though the test suite doesn't do that)

I'm sure the exercise could've been designed a different way, but is it worth it to break existing solutions?

ErikSchierboom commented 5 months ago

I'm sure the exercise could've been designed a different way, but is it worth it to break existing solutions?

I don't think so. I just wanted to discuss some of the design decisions to maybe improve future exercises. I'll post a more detailed reply next week when I'm back at work!

ErikSchierboom commented 4 months ago

Okay, so what I found confusing/did not like was:

The function named parse is not expected to do any parsing at all. It just "wraps" existing information in a struct, which is not really parsing.
The exercise forces the student to a very specific solution, namely one where codons are looked-up from a vector or map. I'd much rather allow the students the freedom to solve the exercise how they like, e.g. by pattern matching. The struct to me is an implementation detail forced upon the user.
Related to the above, the struct/impl setup feels more complex than it could be, where parse (or a better named function like proteins or something) should just immediately return the translated proteins.
I feel that the setup of the exercise makes things unnecessarily harder for the student. I'm not saying that structs, lifetimes and impl blocks are not key Rust concepts, but I am arguing that this exercise does not need them.

Summarizing, I think this exercise has its fair share of problems. I'm not suggesting we redesign the exercise, as that would indeed break existing solutions, but I am writing this as a sort of cautionary warning for when new exercises are being added.

senekor commented 4 months ago

I agree. There are some situations where it might make sense to force a specific solution, if the intent of the exercise is to teach a specific concept. (I like the design of space-age for example.) On the other hand, that's what concept exercises of a syllabus are for... Either way, this exercise doesn't really teach anything, it just forces a specific, weird solution.

What do you think about deprecating it?

ErikSchierboom commented 4 months ago

On the other hand, that's what concept exercises of a syllabus are for...

Indeed!

What do you think about deprecating it?

Let me ponder this a bit!

ErikSchierboom commented 4 months ago

What do you think about deprecating it?

We've talked this over internally, and we feel that it might be best to restructure the exercise. Yes it'll break existing solutions, but it'll make things a lot more sane for future students.

senekor commented 4 months ago

Alright, I should be able to find some time in the evening.

exercism / rust

protein-translation: slightly confusing setup #1937