Jeremie noted that he wanted to stick close to the problem specs, which I hadn’t read, and is a fair point.
I do think the code would be significantly cleaner as a finite state machine though, a lot of the error cases disappear, and its impossible for a client to use it wrongly. The api also teaches the caller about how it should be used.
I feel that by not doing this, we are at best missing an opportunity to guide students down a good path, and at worst, leading them down a bad path.
I don’t have an opinion on that matter, but only three tracks have implemented this exercise to date: C# (17 completions), Elixir (1 completion), and Elm (0 completions). If folks do want to change the exercise, we won’t break very many solutions.
I suspect a proof-of-concept using finite state machines would be illuminating for people including myself. If there isn’t consensus on updating the exercise, that implementation can also be spun out as its own track-specific exercise.
Thanks Andras. Thats a good point, I’ll try and do that. Probably for C# being as Elm and Elixir are a bit more niche, and functional programming is less well known by the average developer.
Is there a process for this? Open a PR in the problem specs repo and then open issues for C#, Elm and Elixir and then open prs for the tracks or something like that?
Open a thread on the forum with the proposed change.
Discuss on the forum until there is a consensus about if and what should change.
Create a PR to modify the problem specs. This PR will require approval from at least three maintainers. This process will go much much much better if three maintainers already signed off on the change on the forum.
Once the problem spec change is merged, the configlet sync tool helps sync from the problem spec repo to other tracks. Each track will be a PR and approval from a maintainer. In general, get prior approval from a track maintainer via the forum prior to opening a PR for that track.
That said, I’m not sure if SleepinessByte was suggesting changing the exercise on the one track, or in the problem spec.
If you intend to change the problem spec, you should propose what exactly you think it should look like on the forum and get consensus before opening any PR.
Basically what Isaac said. If you want to modify the exercise to be a state machine, feel free to do so. I’m not sure the canonical data need to be updated for that, unless there is something obviously wrong with it.
I don’t understand why there would be no error cases. Wouldn’t you need to validate state transitions are valid? Or are you just assuming that only valid state transitions are applied?
The idea of the finite state machine is that the current state (ready, stopped, running) also includes the actions that you can take in that state, and these actions are not exposed any other way, which means that only valid state transitions are possible.
So the only change would be to remove tests related to errors? Given the tracks implementing a finite state machine can freely choose not to include those tests. I don’t see why we’d need to remove those tests.
I suppose you could use the canonical data and errors to validate that the error-causing transitions are not listed in the set of valid transitions. Or you can just opt out of those tests on specific tracks. What @BNAndras said. I don’t think they need to be removed from the problem specs.
That would work, and maybe I’ll do that for the Elm track.
My initial point was to change it for all tracks though
I do think the code would be significantly cleaner as a finite state machine though, a lot of the error cases disappear, and its impossible for a client to use it wrongly. The api also teaches the caller about how it should be used.
I feel that by not doing this, we are at best missing an opportunity to guide students down a good path, and at worst, leading them down a bad path.
That would fundamentally change this exercise. If we were to “force” students to use a state machine when solving this exercise, and if a state machine includes valid actions, we’d probably want to change the exercise to check the valid actions at each state. Simply removing the checks on its own doesn’t nudge/force/direct students towards using a state machine and doesn’t ensure they have a robust solution (ie “includes the actions you can take” or checks that an action is valid). We probably want, one way or other, for the solutions to have something in them somewhere that controls what transitions are/are not valid.
I’m not sold on changing the problem specs and forcing a state machine approach here. This exercise wasn’t designed as a state machine exercise and it does a bunch of other stuff already.
If you want to turn this into a state machine exercise on a specific track, you can use the error cases to check that specific states are not present.
If you want to see a state machine exercise in the problem specs, it might be best to build an exercise specifically designed around that, and not rewrite an existing exercise.
This. I think in Elm this exercise works well as a state machine. I would therefore recommend:
turn off the mentioned tests you lists @ceddlyburge in tests.toml or whatever the file is called.
add new tests locally to the track specific one that are necessary.
update the exercise.
The moment we have a second track also feeling state machine makes more sense, I think we should then include extra tests to the canonical data with a specific tag that they are for state machines, and NOT remove any of the old ones. We could mark them as non-state-machine implementations.
Ok, it looks like we have a plan.
For the record, I disagree with this, for the reasons I’ve already mentioned (I think it doesn’t encourage good code / does encourage bad code).
You don’t have to take my and DJ’s opinions as a final decision ;) If you feel strongly about it, feel free to make a case for it and see if you can convince maintainers that it should be changed.
Problem spec changes need three maintainers so me and DJ certainly isn’t a quorum in either directions.
