Allow array as presenter mapping about application HOT 27 CLOSED

👍

from application.

👎 It's even more obscure than current syntax. :(

from application.

I think it should be objects, then you would register

nette.application:
    mapping:
        *: Nette\Application\DefaultPresenterMapping("App\\", "Module\\*Module\\", "Presenter\\*Presenter")
        Custom: My\Custom\PresenterMapping()
        VeryCustom: @My\Custom\PresenterMapping() # find service in container

ofcourse, the default syntax could replace string for the Nette\Application\DefaultPresenterMapping due to compatibility, so you'd get

nette.application:
    mapping:
        *: App\Module\*Module\Presenter\*Presenter # interpreted as Nette\Application\DefaultPresenterMapping("App\Module\*Module\Presenter\*Presenter")
        Custom: My\Custom\PresenterMapping()
        VeryCustom: @My\Custom\PresenterMapping() # find service in container

and ofcourse, you'd have to implement an interface

class IPresenterMapping {
    public function formatPresenterClass($presenter);
    public function unformatPresenterClass($class);
}

Then PresenterFactory's only job would be to find the namespace in array and call the method.

from application.

@fprochazka Like! :-)

from application.

@fprochazka That is beautiful 👍

from application.

👍

from application.

Who's gonna implement it guys? :)

from application.

@enumag maybe something like this is more readable than array:

nette.application:
    mapping:
        *: App\[Module\*Module]\Presenter\*Presenter

@fprochazka is not it overengeneered?

from application.

@dg It's not bad but I slightly prefer array. In most cases I like to keep things simple instead of inventing new syntaxes.

from application.

@dg I don't think so, because in the current way, you have to invent "new magic syntaxes" to add mapping options. But "my way", you can just code it in plain PHP and mainly you can easily write a unittest for it, without having to pipe it through PresenterFactory first.

from application.

@dg: please, do not create new metalanguages. I'd like more the verbose php solution, than something hidden in the strings. It's much more difficult to do tools which should support it (intellij-neon plugin), etc.

from application.

It is not metalanguage, but domain-specific language.

from application.

@dg: please, do not create new domain-specific language. Reasons above.

from application.

It already exists. App\Module\*Module\Presenter\*Presenter.

from application.

I don't claim I like it, I wanted to point out I would like it wouldn't get any new features.

from application.

@dg well yeah, but it's way too magic (for beginners) imho. I have to study the code every time I start a new project. Mostly changing the first namespace is enough but anything more complex and you have to look at the code or directly extend the PresenterFactory to override it, as it's often simpler than try to hack the domain-specific language.

I'm curious, what is your experience with this from trainings and consultations?

from application.

On training I am using this infographic and I think that people understand:

but anything more complex and you have to look at the code or directly extend the PresenterFactory to override it,

In what cases?

from application.

@dg basically anything that involves the *, adding and removing the module from the mask for example - I always have to check that how it works. Not that I would start project that often.

from application.

I started thinking about implementation. Is this the behaviour you need? Do I understand it correctly?

test(function () {
    $factory = new PresenterFactory;
    $factory->setMapping([
        '*' => ['App\\', 'Module\\*Module\\', '\Presenter\\*Presenter'],
    ]);
    Assert::same('App\Module\Jupiter\Presenter\RedDwarf', $factory->formatPresenterClass('Jupiter:RedDwarf'));
    Assert::same('App\Module\Universe\Module\Jupiter\Presenter\RedDwarf', $factory->formatPresenterClass('Universe:Jupiter:RedDwarf'));
});

test(function () {
    $factory = new PresenterFactory;
    $factory->setMapping([
        '*' => ['App\\', '*Module\\', '*Presenter'],
    ]);
    Assert::same('App\Universe\Jupiter\RedDwarf', $factory->formatPresenterClass('Universe:Jupiter:RedDwarf'));
});

from application.

@achse Actually no. Here is what I want.

test(function () {
    $factory = new PresenterFactory;
    $factory->setMapping([
        '*' => ['App\\', 'Module\\*Module\\', '\Presenter\\*Presenter'],
    ]);
    Assert::same('App\Module\JupiterModule\Presenter\RedDwarfPresenter', $factory->formatPresenterClass('Jupiter:RedDwarf'));
    Assert::same('App\Module\UniverseModule\Module\JupiterModule\Presenter\RedDwarfPresenter', $factory->formatPresenterClass('Universe:Jupiter:RedDwarf'));
});

And it works with the current implementation of PresenterFactory. The only problem is that I have to hack the mapping array using reflection.

from application.

Ok, there is a proof of concept solution (I moved from red to greeen). Plase give me some feedback if I didn't miss any case or if there any missing functionality.

Then I'll do some code cleaning & refactoring. I would like to extract the logic of parsing into some "PresenterMappingParser".

from application.

I like @fprochazka solution best. And implementation would be much more simplier than in PR #119

from application.

I'd like to ask a question:

How much do you have to do, to write a test for the mapping configuration with current syntax? VS How much you'd have to do, to unittest a solution, where the mapping logic would be separated from PresenterFactory?

In terms of infrastructure and ensuring that what you've tested is in fact what the app is using... will you be loading the config and getting the presenter factory from DI Container? Will you be copying the mask from config to unittest? You'll also have to mock arguments of PresenterFactory. Etc, etc...

Imho the way I've proposed is cleaner and promotes testability.

from application.

I assume discusion here have nothing to do with my PR and it's still about "@gd -like" syntax versus. arrays as @fprochazka proposed.

When you came with conclusion I can rewrite my PR. With current logic, I tried to be able to do it both ways, but the complexity of that is imho awful.

from application.

I've been testing for a while now something like this: https://gist.github.com/xificurk/3eb69d7f3f7dd44abbcd
It works well for my needs, but I'm still not completely satisfied with the current code.

My main motivation was that the default presenter mapping in Nette is pretty "dumb", here are two problems I ran into:

1. Default mapping works only for top-level modules, but not submodules independently. I bundle presenters to corresponding composer packages and I would like to reference them as presenters inside submodule 'Vendor:Package'. To make this work with default mapping I would need to have exactly same namespace structure for every package (which is not always practical) and CompilerExtension in every package would need to set the global mapping for 'Vendor' module (which is ugly).
2. For some applications I need to do a minor customization of the installed presenter, i.e. I would like to swap Vendor\Package\FooPresenter for e.g. App\FooPresenter without breaking links etc. This can't be done with default mapping at all.

from application.

@achse Thanks for solving it!

@dg Is it possible to have this in 2.3 as well? There is no BC break so it should be fine.

from application.

@enumag done

from application.