Sync robot-simulator #784
Sync robot-simulator #784
Conversation
Also spreads tests out to indepent test methods, uses only the defined interface from problem spec and still is compatible with existing solutions.
README translates to instructions nowadays
mk-mxp
added
x:action/sync
|
This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested. If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos. For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping |
|
@fejan-malek I cannot add you as a reviewer, because you are no maintainer yet. Anyways, what do you think about a fluent interface here? |
|
@mk-mxp I agree with using a fluent interface because it helps write clean code, better code readability, and reduces code complexity. What do you think about the existing solution already submitted in this exercise? Is it gonna affect these changes? |
@fejan-malek Not using an implemented method does not invalidate any existing solution. |
|
I've been trying out several variants for a fluent interface. The problem is, that all the benefits of the fluent interface are hidden in the tests. The student experiences nothing but "additional work to return The exercise becomes rather "boring", with only conditionally adjusting values. "Boring implementation" can be a quality of good design, but is discouraging for students at Exercism. There would be only one challenge left in the exercise. The string splitting and looping disappears. These two points made me decide: The exercise shall stay as close as possible to problem specifications. Fluent interfaces are a concept to introduce in an extra exercise, most likely a PHP track only exercise. |
|
Note that I didn't took time to take a look yet, I will probably look into this tomorrow. About fluent interfaces I'm not in favour of having those: https://ocramius.github.io/blog/fluent-interfaces-are-evil/ That said it could be great to introduce them in an exercise just so the students know they can write code like this and understand how to read code like this. |
There was a problem hiding this comment.
Thanks for the changes. I will take a look into #783, I think it would make sense to think about the re-design first so newly published solutions will remain correct in the short future.
This takes the exercise closer to problem specifications
This breaks all existing solutions.
This provides proper encapsulation. Also breaks all existing solutions.
- Remove dead code - Adhere to interface of student stub - Type annotations, typos, code order
OK, that's pedantic. But computers are pedantic.
Sync'ed according to #781. Changes tests heavily in structure,
but notand in content. Sonore-testing against existing solutions required.StillRe-design as discussed in Re-designrobot-simulator#783I did find a way to do a re-structuring without altering the interface relied on by existing solutions. There are now no extra methods used, that I think were older than the current
instructionsproperty in problem spec.The exercise is marked as
topic: ['oop']inconfig.json, which hints on why there were specialised methods to change the internal state. And why ourexample.php(and an additional test) does method chaining (fluent interface design).To stay as close to the problem spec as possible, the OOP-isms are seemingly superfluous. A fluent interface design deviates totally from the string style
instructions. On the other hand one could transform the string input into the fluent interface while writing the tests, and also allow immutable instances as a more modern approach that way. But still, I think such OOP-isms should only be in additional PHP track exercises.