Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul basic-syntax and lasagna #607

Merged
merged 25 commits into from
Feb 17, 2024

Conversation

mk-mxp
Copy link
Contributor

@mk-mxp mk-mxp commented Jan 30, 2024

This very first concept and its concept exercise touch a lot of concepts coming later.
These must be introduced as much as necessary to understand the code and the requested solution, but not more.
So we have many short introductions and a bit more depth for the about.md. But what is "Basics" or "basic-syntax" then?

The important decision guiding document (according to exercisms docs) is design.md, which was a copy from the Ruby track and not adjusted for PHP Basics. I analyzed the exercise (as this must remain mostly the same to not invalidate solutions) and re-wrote design.md to inform further decisions. And answered the question "what to teach here" from this perspective.

I then added missing conceptual information to about.md for little experienced programmers and condensed that document for introduction.md to address medium experienced programmers. I kept the chapter on code organisation in about.md and added a chapter on simple debugging. As I think this helps people who are stuck also, I added var_dump() to hints.md.

I focused the exercise stub on doing the function bodies for the tasks from instructions.md by adding method definitions without body. As outlined in design.md I think it is not "basic syntax" to define the interface of methods, that is "user-defined functions" and "class basics".

I intentionally chose comments for the function bodies and not throwing exceptions, as that would add another concept to touch.

Finally I added more value to the exercise tests by making their feedback more helpful and guiding.

  • The concept has its own concept exercise
  • The concepts configuration integrates nicely into the concept map
    • The name and subtext (blurb) read well together
    • The concept is connected to the prerequisite concepts as planned (has none)
  • The concept follows exercism guidelines
    • about.md for in-depth discussion of the topic
    • introduction.md with concise information to address a medium experienced programmer who is new to PHP
    • introduction.md links to about.md for those who need / want more information
  • The concept exercise follows exercism guidelines
    • design.md outlines goals, learning objectives, scope, unlocked concepts, prerequisites
    • Re-use introduction from concept
    • instructions.md uses steps to drive towards the solution
    • hints.md unstucks programmers without handing out the solution, mostly by refering to helpful sources to read
    • hints.md links back to about.md where reasonable
  • The concept exercise uses only concepts already introduced
  • The concept exercise adheres to PHP track consensus:
    • Use classes, not functions
    • Names are in unified casing (classes PascalCase, methods camelCase, variables snake_case)
    • Strict types in tests (and optionally in exemplar), but not in students files (the concept to be mastered is still missing)
    • Avoid named arguments to allow students to name parameters as desired
  • The tests make sense and are helpful to master the exercises
    • Order of tests matches the order of instructions
    • Failing tests give helpful information

Adds to #578

@mk-mxp
Copy link
Contributor Author

mk-mxp commented Jan 30, 2024

I'm not sure about re-arranging the tests to follow Arrange, Act, Assert or Given, When, Then with additionally separated out input and expected values. The arrangement would allow easier implementation of a test generator for practice exercises, students would get used to seeing a test arrangement recommended for unit testing and many implementation details look much nicer when separating the data from the phases.

Current code:

    public function testTotalElapsedTime(): void
    {
        $lasagna = new Lasagna();
        $this->assertEquals(21, $lasagna->totalElapsedTime(layers_to_prep: 4, elapsed_minutes: 13));
    }

Arrange-Act-Assert:

    public function testTotalElapsedTime(): void
    {
        $input = [
            'layers_to_prep' => 4,
            'elapsed_minutes' => 13,
        ];
        $expected = 21;

        $subject = new Lasagna();
        $actual = $subject->totalElapsedTime(...$input);

        $this->assertEquals($expected, $actual);
    }

Also I would suggest using a common naming scheme for the tests. E.g. I usually use $expected, $actual, $subject (rather than $subjectUnderTest or $sut) like shown in the example above.

@mk-mxp mk-mxp mentioned this pull request Feb 14, 2024
Copy link
Contributor

@homersimpsons homersimpsons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall a great improvemen! I opened some threads on things I think we can improve.

Do not hesitate to discuss them.

concepts/basic-syntax/about.md Outdated Show resolved Hide resolved
concepts/basic-syntax/about.md Show resolved Hide resolved
concepts/basic-syntax/about.md Outdated Show resolved Hide resolved
concepts/basic-syntax/about.md Show resolved Hide resolved
concepts/basic-syntax/about.md Show resolved Hide resolved
exercises/concept/lasagna/LasagnaTest.php Outdated Show resolved Hide resolved
exercises/concept/lasagna/LasagnaTest.php Outdated Show resolved Hide resolved
exercises/concept/lasagna/LasagnaTest.php Outdated Show resolved Hide resolved
exercises/concept/lasagna/LasagnaTest.php Outdated Show resolved Hide resolved
@mk-mxp mk-mxp force-pushed the overhaul-basic-syntax branch from 8daae6f to a8dd192 Compare February 17, 2024 11:22
@mk-mxp mk-mxp self-assigned this Feb 17, 2024
@mk-mxp mk-mxp added the x:action/improve Improve existing functionality/content label Feb 17, 2024
@mk-mxp mk-mxp added x:knowledge/intermediate Quite a bit of Exercism knowledge required x:module/concept Work on Concepts x:module/concept-exercise Work on Concept Exercises x:type/content Work on content (e.g. exercises, concepts) x:size/medium Medium amount of work x:rep/medium Medium amount of reputation labels Feb 17, 2024
@homersimpsons homersimpsons merged commit 2235dfd into exercism:main Feb 17, 2024
12 checks passed
@homersimpsons
Copy link
Contributor

Thank you very much for all those improvements !

@mk-mxp mk-mxp deleted the overhaul-basic-syntax branch February 17, 2024 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:action/improve Improve existing functionality/content x:knowledge/intermediate Quite a bit of Exercism knowledge required x:module/concept Work on Concepts x:module/concept-exercise Work on Concept Exercises x:rep/medium Medium amount of reputation x:size/medium Medium amount of work x:type/content Work on content (e.g. exercises, concepts)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants