Namespaces and PSR-4 compliance

[arueckauer]

One of the points I am missing in the exercises of this track is the concept of namespaces (PSR-4 compatibility). Therefore, I would like to discuss and propose the following change.

Suggestion

Each exercise separates actual business logic from tests by having separate src and test directories. The README.md stays in the exercise's root folder. The example solution (and coming stub file) are going into src and the test file into test.

The terminating class name corresponds to a file name ending in .php. The file name MUST match the case of the terminating class name.

Since there is no definition for declaration of functions pertaining to the filename, two possible solutions are crossing my mind. Either name the file functions.php or exercise-name.php - both residing within the exercise's namespace. Here I am interested in suggestions, since I haven't used a file collecting various functions in years.

Example

Taking Bob as an example, this would result in the following structure:

• /src → namespace Exercism\Bob
  • Bob.php (stub file)
  • example.php (example solution)
• /test → namespace ExercismTest\Bob
  • BobTest.php
• README.md

Looking for feedback

My intention is to provide an even greater learning experience for the student within a modern (meaning current) environment.

It would be great to get some feedback on the suggestion over all and possible side effects I might not have fully considered yet (like implications on test automation etc.). Thank you!

[dstockto]

I really like the introduction and consistent use of namespaces for the PHP track. As a test, I went ahead and modified my old bob code to follow the example above. I added a setupBeforeClass that performed a "require_once" on "../src/Bob.php". I also had to update my signatures of setup and setupBeforeClass to include the void return declaration, but after that it worked.

I also tried this on the Accumulator exercise (function based). In addition to adding the directories and adding namespaces, I had to also add the use statement:

use function Exercism\Accumulate\accumulate;

Similarly to Bob, I also added a setupBeforeClass method to require_once the file from ../src/Accumulate.php. Once those were in place, I had all but one test passing. That test was the one that uses a static function to feed into accumulate function. I had to update the code to include the namespace in the string:

    public function testAccumulateUsingStaticMethod()
    {
        $this->assertEquals([5, 6], accumulate(['Hello', 'World!'], '\ExercismTest\Accumulate\Str::len'));
    }

As a side note, I also noticed that all the tests that extend from \PHPUnit\Framework\TestCase do not seem to include the leading backslash. It still works, but I think PHP has to see if it knows about \Exercism\Accumulate\PHPUnit\Framework\TestCase and then it falls back to \PHPUnitFramework\TestCase.

So, with all this, here's my take: I like using namespaces, and I fully support using them in the tests and the code. Since PSR-4 is about autoloading though, I don't think we're going to realistically do much about that. Unless we can introduce an autoloader (composer or custom) at the root of the ~/Exercism/php directory, it means we'll need one inside each exercise.

If we introduce an autoloader at the root level (assuming that's even possible) then we need to presumably have either a root level config for tests (not sure if this would be feasible) or a test config in each exercise that loads the autoloader which then allows for loading the code and tests automatically. Since the exercises are not capitalized and we're introducing a src and test inside each, I think it means that if we were to go the PSR-4 composer route, then we'd need a composer.json at the root that lists every exercise's namespace (code and tests) mapped to the appropriate directories. I don't know for sure this is possible, and even if it is, I'm not sure it's worth it.

The next potential option (again, not sure it's feasible) would be to introduce a custom autoloader in each exercise. Of course in order to use the autoloader, it needs to be loaded and registered. This means phpunit.xml files in every exercise and updates to the instructions of each exercise about how to run the tests. I'm not sure this is worth it either.

With those two out of the way, a third option is to introduce an autoloader that we load and register within each test file. This would mean we don't need the phpunit configs, but creating and loading an autoloader for each exercise just so it can in turn autoload the class we're testing seems a bit silly and probably overkill.

This leads back to either requiring the file we want to test in the test file, either directly as has been done in most of the tests currently (and is complained about by many static analyzers) or we load the file in the setupBeforeClass method which would need to be introduced to every test file where it doesn't exist already.

As much as I'd like to have autoloading "just work", I don't think the structure or the idea of the separate exercises in Exercism lends itself to having an over-arching autoloading solution and introducing an autoloader for each exercise means instead of T -> S we have T -> A -> S where T is our test, A is the autoloader and S is our SUT.

So to wrap up the wall of text with a TL;DR - I like the use of namespaces, and think we should introduce them to the exercises. I don't think autoloading is going to work very well within the structure of ~/Exercism and how all the exercises are essentially completely isolated.

[arueckauer]

Thank you for that thourough testing and feedback! That is very valuable, I appreciate it.

Yes, some tests will need refactoring. AccumulateTest is a very good example. Declaring multiple classes within one file is a violation. They should reside in their own files. AccumulateTest can then require and import these. At least that is how I would solve it. Haven't used custom callables in test so far. If there is a better practice, I'm open to it.

The issue with the missing leading backslash on PHPUnit\Framework\TestCase I would simply solve by using an import.

<?php

namespace ExercismTest\Bob;

use PHPUnit\Framework\TestCase;
use Exercism\Bob\Bob;

class BobTest extends TestCase
{
    // [..]
}

I agree, there is currently no clean solution to autoloading and is not part of this change. PSR-4 is only relevant to the definition and usage of namespaces.

@petemcfarlane Is there something you would like to add? Not sure, if there are other active contributors, have not received any reaction in Slack.

Otherwise I would go ahead and start working on a PR.

[arueckauer]

One point I have been missing so far is the introduction of namespaces to the student. I am unsure whether to start with the first exercise or another. We could add the namespace to the Hello-World stub. And probably should for a) consisteny and b) help the student get familiar with the structure from the beginning. I imagine a explanatory note and/or exercise for namespacing. This does not have to be Hello-World, but at some point, where just an empty stub file is provided, the student should be able to set the correct namespace by him- or herself.

[petemcfarlane]

Sorry to not be more involved in this issue, I've been on annual leave until today.

I feel that exercism should have a very low barrier to entry where possible, but at the same time should introduce concepts and best practices to students where possible. Typically, I think nearly all modern PHP applications I have worked on have used Composer for dependency management and autoloading - though that's just my experience, does anyone have contrary experience? We could bite the bullet and just introduce Composer as a requirement. Heck - we could even introduce exercises that specifically show how to use third party dependencies if we do that!?

Whilst most modern (and all PSR-4 conforming) applications would use namespacing combined with autoloading, there is a judgement to be made on how much extra work or overhead is there to think about/juggle these concepts, the value of using namespaces - along with the core exercise or problem that the student is trying to solve. It's a balance between exploring and learning the language, and solving a particular algorithm or problem. To me the main benefit of namespacing is a clear separation and ordering of logic, and PSR-4 provides a fantastic, easy (consistent and predictable), interoperable way to load .php files. But when all we're loading (often) is the exercise file - is it really adding much value?

If you just want an exercise to explicitly show how namespacing works, how about creating a new exercise which highlights namespacing/PSR-4 rules using various files, classes (and functions)?
Or we could adapt one, two or several exercises to use namespaces, and leave some untouched (Though that might make the Makefile trickier to manage, but as far as I see it, the make file is only for our benefit as specific PHP track maintainers/contributors - we can bend it to suit our needs.)

Another idea I had, which I didn't see you explicitly mention in the earlier conversation, is to create a phpunit.xml file in the root of the problem, and specify a bootstrap.php file which could load the src and test files, (possibly in a naive require_once fashion, recursively iterating over files in both directories, or explicitly loading each required file). This would require two new files in each exercise, a bootstrap.php or autoload.php, and (probably, though not necessarily) the phpunit.xml configuration file.

I like the idea of having an example directory, but I don't know how the exercism cli tool knows not to send the current example.php file to the user, perhaps something to do with https://github.com/exercism/php/blob/master/Makefile#L12 maybe?

[arueckauer]

Hope you had a great vacation! 😊

Although I acknowledge the almost omnipresence of composer, I am hesitant introducing it into exercism, since it is not native to PHP.

The main value from adding namespaces is teaching a good practice, that will become most beneficial in a context outside of exercism. By introducing namespaces we give the students an opportunity to practice and understand the concept. If we use a one explicit exercism, the learning effect is much less. If there is one exercise introducing namespaces and following exercises building on that knowledge, it would be much better.

These are great suggestions on how to solve possible issues with makefile and others. I believe, we need to first agree on how (far) we want to introduce namespaces. Everything else is dependent on that decission.

My suggestion: Leave hello-world as-is. Introduce namespace in Gigasecond exercise with a prepared stub file and the exercise after that. From the third exercise on, provide only empty stub files, having the student declare namespaces him-/herself.

Thoughts?

[petemcfarlane]

Yeah I like your final suggestion. Do you think we can gradually introduce namespaces one exercise at a time in individual PRs? I guess we need to figure out how to run the example in travis, and find out why it's not included by the exercism-cli, @kytrinyx do you know about this or who we should speak to?

[arueckauer]

I would like to first get an overview of all required changes. Can we open a separate branch for that? There we could build a prototype (for makefile, travis setup etc.) and collaborate. Doing so will not affect production.

[kytrinyx]

find out why it's not included by the exercism-cli

By default any file with example (case insensitive) in the name will be excluded when delivering files from the Exercism API. This is because the example solutions should not be delivered.

You can override what will be ignored by specifying an ignore_pattern key in the config.json at the root of the repo. This will then take precedence.

[arueckauer]

I like the idea of having an example directory

Yes, I believe a separate directory for the example code has the benefits of a clear separation and allows easier management for cases with multiple classes (files). The structure would then look like the following.

• /example → namespace Exercism\Bob
  • example.php (example solution)
• /src → namespace Exercism\Bob
  • Bob.php (stub file)
• /test → namespace ExercismTest\Bob
  • BobTest.php
• README.md

A discussion @dstockto and I had in #288, was about the filenames for exercises using functions only. The PHP-FIG has no rule on naming function files. To distinguish between classes and function files, I like David's idea of naming the stub file function.php and the test file FunctionTest.php.

@petemcfarlane Would that be okay with you?

[petemcfarlane]

I don't mind the functions.php file - it seems to be a clear naming convention at least.

I've also seen (and quite liked) the file take on the namespace as a file name, so that would lead to

• /src/Acronym.php -> namespace Exercism\Acronym

<?php

namespace Exercism\Acronym;

function acronym($phrase)
{
}

What do you think to that approach?

[arueckauer]

Yes, that's possible. Although it introduces two issues: It makes it impossible to

1. differentiate between files containing a class and files containing function(s) and
2. to define a class Acronym in that namespace.

[petemcfarlane]

Ok good point. I think situation 2 is unlikely in our exercise project but your first point is reasonable. Let’s go with functions.php then? 👍

[neenjaw]

There have been several breaking changes in the directory structure, the intended usage of the exercises (via website code editor) in preparation for Exercism v3. Are there plans to move foward with this, adapt them to v3?

This affects both #287 and #288 as they depend on this being finalized as well as each of them to be updated for the new directory structure.

As a side note, matrix tests (like using a data provider) are not straightforward to use in the website from a UI perspective (though I think the Go track with @ekingery is doing some work with these), so they are slightly discouraged.

[arueckauer]

Thanks for the follow-up. It's exciting to see all the changes coming to the repo. 👏🏻

Wherever possible I would encourage the usage of best practices for a particular language and library (data providers). Luckily/unfortunately I am very busy and it does not look like I have the time to make a PR for v3 in the upcoming weeks. So, I make room for anyone who would like to pick it up from here.

[neenjaw]

Thanks for your reply! I'll see how things with @ekingery and the go-track develop with their testing strategy, and keep it in mind to migrate to data providers in time.

[ekingery]

Thanks for your reply! I'll see how things with @ekingery and the go-track develop with their testing strategy, and keep it in mind to migrate to data providers in time.

This might already be understood, but just in case not - I would not migrate to data providers until someone can do the work to support them in the test runner. It's possible, but really complicated things with the Go test runner and it still isn't in great shape. I'm not yet convinced the test runner and associated consumption of the results.json by the website is really compatible with matrix / data provider tests. Which I think is what @neenjaw is waiting to confirm. Hope that helps!