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

Discussion: Agree and implement CGL #658

Open
tomasnorre opened this issue Mar 5, 2024 · 3 comments
Open

Discussion: Agree and implement CGL #658

tomasnorre opened this issue Mar 5, 2024 · 3 comments

Comments

@tomasnorre
Copy link
Contributor

After briefly talking with @mk-mxp about coding guidelines #652 (comment) we agreed to split this out in a separate issue to follow up on this later.

We should agree on it first, though. I think tools like rector or other static code tools would be able to streamline this for us, when we have an agreement on what should be when.

@mk-mxp
Copy link
Contributor

mk-mxp commented Mar 5, 2024

What I have collected so far, from old issues, the exercise code I have seen and my own favourites:

  • PSR-12 as enforced by CI.
  • No line length limit, but for readability try to keep it under
    • 80 chars for students code stubs.
    • 60 chars for test code.
    • Comments (especially DocBlocks) should wrap at 80 chars, too.
    • Keep in mind: in online editor, there is very limited choice for arranging code shown.
  • Use snake_case for functions, camelCase for methods / properties and PascalCase for classes. The casing for names of variables in student facing code (mostly parameters) is not fixed - I'd prefer camelCase, too. And my wish for constants: ALL_UPPERCASE.
  • Use classes, where the instructions would allow functions or methods (there are of course exercises using functions intentionally).
  • Despite having discussed namespaces (in Namespaces and PSR-4 compliance #283), the way Exercism works strongly discourages namespaces / PSR-4 directory structures. I would use namespaces only in (yet not existent) exercises on namespaces and no PSR-4 directory structures at all (they make no sense here).
  • Stick with ASCII for all names in code and use Unicode in strings sparsely. Provide comments on what type of Unicode codepoints you used (normalized to a composition schema, graphemes, ...) or encode the Unicode code points (\u{[0-9A-Fa-f]+}).

@tomasnorre
Copy link
Contributor Author

tomasnorre commented Mar 5, 2024

I would personally go for camelCase for functions too, like getData() and not get_data.
I have used camelCase in all the PRs I have created recently.

I think it should be possible to have the phpcs scan for all this, in most IDEs. But would have to check when it differers in different files. But perhaps it is possible as we stick to conventions of e.g. example.php in the .meta folder.

@mk-mxp mk-mxp changed the title Agree and implement CGL Discussion: Agree and implement CGL Jun 17, 2024
@mk-mxp
Copy link
Contributor

mk-mxp commented Sep 24, 2024

For test files, readability in the online editor is important. In addition to 60 chars line length:

  • All tests should follow Arrange-Act-Assert pattern (using $input and $expected variables)
  • Have inlined subject creation (not hidden in setUp() method)
  • Use no constants or other instance variables to avoid code duplication

When a test fails, these rules help to have all of the code executed in one box directly below the failure message. For CLI users it makes no difference, as we do not teach "PHPUnit for production use".

Edit: Add constants and other instance variables

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants