Re-design `robot-simulator`

[mk-mxp]

The current exercise design has many problems:

• Students must implement constants for directions with no TDD style guidance - since PHP8 this could be an enum
• The test accesses position and direction properties directly, but protected fields are provided in the stub
• The class is not named like the file - Robot vs. RobotSimulator.php
• The tests expect methods to move the robot, while problem spec only has instructions (will be solved in Sync robot-simulator #784)
• The tests are clumped together with no clear separation between test cases

Problem specifications do not ask for a certain implementation. They define the problem statement as open as possible. Our design also should allow as many solutions as possible.

Current stats:

Robot Simulator

Started 672
Attempts 4,498 (avg. 6.7)
Completions 446 (66.4%)
Mentoring requests 44 (6.5%)

No discussion about these changes

• When we break things, rename the class to match the file name.
• Test no extra methods to move the robot - instructions() only or fluent interface only. (see Sync robot-simulator #784)
• Structure tests according to problem spec - Nested test cases share the test name prefix of the group. Each test case is a test method.

Directions

Suggestion 1: Strings only

This leaves all possibilities open. Mentors can suggest enum or such to students. But enum is optimal for interfaces, internal enum usage just complicates things.

Suggestion 2: Provide enum stub

In addition to the tests from problem specifications we test the enum implementation first. When that is implemented, the actual problem is tested for. In these tests we must use the enum type, too (like the constants now).

This forces students to use enum. It raises the bar for students not familiar with that. But enum would be a good choice in a real life scenario.

Suggestion 3: Provide ready-made enum

As before, this forces students to use an enum type. But the level of required knowledge is lower (only usage is required). But still, it makes no sense to work with other types internally then.

Which way to go? Other suggestions?

Accessing state

I think testing the state by accessing fields was done to force students to implement a state encapsulating solution with __get() or such. The exercise is marked as topic: ['oop'] in config.json, which hints on why it may have been designed like this.

To stay as close to the problem spec as possible, the OOP-isms are seemingly superfluous. Accessing protected properties via __get() could be avoided OOP-style also.

Suggestion 1: public properties

This is closest to the problem specification and the easiest to implement. But it violates fundamental rules of OOP - hide the details behind methods.

Suggestion 2: get methods

Ask to implement getter methods to access the internal state. This also is easy to do. It makes the properties readonly from the outside but mutable from inside.

Suggetsion 3: state method

Combine the internal state to an array or result object holding all internal values we are interested in. Looks a bit artificial, but we have that in another exercise (Solution) already and it is good OOP practice.

[homersimpsons]

Directions

Reading https://github.com/exercism/problem-specifications/blob/main/exercises/robot-simulator/canonical-data.json I think we should go with the string solution.

If the canonical data would be divised in a list of instruction: instructions: [R, A, A, L] that would make sense to use an enum. But here I see the "parsing" as part of the exercise.

Accessing state

I think the most common here is to have a getter so I would be in favour of this.

Having another object to represent both position and direction could be interesting, we could use it in construct and in a getter. But I think this would add unnecessary complexity.

Conclusion

I would vote for the following interface:

class RobotSimulator {
  public function __construct(private array $position, private string $direction) {}

  public function instructions(string $instructions): void {}

  public function getPosition(): array {}
  public function getDirection(): string {}
}

The student still is free to declare the enum he want for increased typing / validation.

[mk-mxp]

Thanks for your feedback. I'm totally OK with getters and string for directions.

I won't add private to make use of constructor promotion in the stub - and also won't add class properties at all. This leaves students the most mental freedom to implement internally as they want. And leaves room for mentors to explain "constructor promotion", "private, protected, public properties" etc.

Also I'll include it all in #784 together with the missing type specifications requested. @homersimpsons If you have no time for reviewing again: I'll merge it on Monday then.

[homersimpsons]

Thanks for your feedback. I'm totally OK with getters and string for directions.

Perfect let's go vith this then.

I won't add private to make use of constructor promotion in the stub - and also won't add class properties at all. This leaves students the most mental freedom to implement internally as they want. And leaves room for mentors to explain "constructor promotion", "private, protected, public properties" etc.

I'm okay with this

Also I'll include it all in #784 together with the missing type specifications requested. @homersimpsons If you have no time for reviewing again: I'll merge it on Monday then.

I will have time to review it today, If you do this accross the week-end I won't reall be available but could try to get a glance. In anycase I'm okay if you merge it on monday even without review.