application
application copied to clipboard
nette
Generalized Route::getTargetPresenter()
Allow using this optimization for other IRouter implementations.
Ideas for better names of anything are welcomed.
We may implement the getTargetPresenters as well it for RouteList as well.
function getTargetPresenters()
{
if ($this->cachedRoutes === NULL) $this->cachedRoutes = $this->buildCache();
return empty($this->cachedRoutes['*']) ? array_keys($this->cachedRoutes) : NULL;
}
Sorry about previous comments. Without further change in RouteList it is major BC break for multiple presenters in single route?
@mishak87 Sorry, I have no idea what you mean. Please rephrase. This feature should be entirely without BC break.
Original: FALSE skips adding anything, NULL adds default values and string one presenter if the item is Route instance.
Change: Adds interface returning multiple routes and removes FALSE branch while also renaming method to plural.
I still don't know what you are talking about. Are you suggesting something? Informing me of something? Are you stating facts?
He means that you changed getTargetPresenter -> getTargetPresenters and it's return value. But getTargetPresenter is internal method, so it is not BC break.
@dg Does this makes sense to you? Should I finish the implementation (e.g. write tests)?
Even after more than one month I still think this is a great idea. Except for the interface name, that is terrible. cc @dg
That would solve the ugly IFixedTargetPresenters name. But it would be a nasty BC break. It would actually be so large BC break that if we choose to do it, we may as well break other things (I have few ideas).
I would prefer to avoid BC break. This method can be in interface in v2.3 commented out and checked via method_exists.
Yeah and it also does not have tests so we don't know whether it actually work. And I don't feel like writing them right now.
There is a potential issue with cache invalidation when combining multiple RouteList, i.e. the following test will fail:
$listA = new RouteList();
$listB = new RouteList();
Assert::same([], $listA->getTargetPresenters());
Assert::same([], $listB->getTargetPresenters());
$listB[] = new Route('about', 'About:default');
Assert::same(['About'], $listB->getTargetPresenters()); // this is OK
Assert::same(['About'], $listA->getTargetPresenters()); // this will fail
One possible solution may be to disallow (throw exception) modifying RouteList once first URL has been generated (or more precisely once the cache is built).
Yes. It would be awesome if anyone could finish this without me (I don't currently have the mental model for this ready anyway) but I can finish this if no one else finds the time.