@tsteur opened this issue on June 16th 2015

I hope there is not already an issue about this since I can remember we had this topic already once re Plugin\Manager or so.

See comment here: https://github.com/piwik/piwik/pull/8045#discussion_r32403167 where we have a class like Piwik\Measurable\Type\Manager. We already have a couple of classes like Manager (we know Manager is not a good name see eg http://www.slideshare.net/pirhilton/how-to-name-things-the-hardest-problem-in-programming but it's not about that particular name). There are suggestions to name such classes eg - Piwik\Measurable\Type\Manager - Piwik\Measurable\Type\TypeManager - Piwik\Measurable\Type\MeasurableTypeManager

to not having to write eg use Piwik\Measurable\Type\Manager as TypeManager.

I usually just use it in the code directly like this new Type\Manager(). This way I do not have to write the use ... as ... and it is still clear what is meant. So just Manager is usually ok for me personally but I can understand that it can be sometimes confusing to find the correct Manager class immediately when searching for the class name in eg PHPStorm. Still I personally prefer just Manager. For example it also allows us to change the wording Type without having to refactor the class TypeManager etc.

Goal of this issue is to maybe agree on a naming issue to have a consistent naming in Piwik and avoiding having discussions about this in PRs.

If we name the class eg TypeManager where would be put "subclasses" of it. In namespace Piwik\Measurable\Type\Manager\Whatever* or Piwik\Measurable\Type\TypeManager\Whatever\*?

In this case there might be also many Type classes. So would it be like Piwik\Measurable\MeasurableType\MeasurableTypeManager\Whatever\*?

@mnapoli commented on June 16th 2015

This is the issue for the plugin manager problem: #7656

The thing with Plugin\Manager is that it's definitely not as fast as typing pm and autocompletion will take care of everything (including inserting the use … statement). Same with all the "go to" shortcuts of the IDE (e.g. typing Maj + Maj). Having the use … handled entirely automatically by PhpStorm is awesome.

Another issue is that it means importing the whole namespace, whereas having the list of imported class at the top of the file is very useful to get an idea of how much this class is decoupled/coupled to other classes (it serves as a good indicator IMO).

I don't have so much of a problem with the "manager" word though (even though it's not the main point of this issue). Naming in Piwik is currently source of many problems and we are on the opposite side of DDD. So I'd rather go with dull yet consistent names because it would be more useful to the project, and it would be simpler to plugin developers, contributors, etc. That's why I argue for DAO or Service names even though I wouldn't necessarily use those in other projects.

Generally I like names that describe what we are dealing with. new Manager doesn't really mean anything. There is no such thing as a manager (well, not in our code base :) ). However PluginManager definitely means something.

For example it also allows us to change the wording Type without having to refactor the class TypeManager etc.

That's the only advantage I can see, but to me it's far fetched. On the contrary I find it easier to rename TypeManager rather than Manager because Manager could appear everywhere, searching for this returns way too much noise.

If we name the class eg TypeManager where would be put "subclasses" of it

Why would there be subclasses?

@tsteur commented on June 16th 2015

The thing with Plugin\Manager is that it's definitely not as fast as typing pm and autocompletion will take care of everything

So far that's the only advantage I can see and mentioned as well before but I personally don't use it that way. Others might do.

Another issue is that it means importing the whole namespace, whereas having the list of imported class at the top of the file is very useful to get an idea of how much this class is decoupled/coupled to other classes (it serves as a good indicator IMO).

That's true. I'm not sure if it's that important though. You can usually also see it by having a look at the constructor or you can usually also see it by other indicators or when scrolling over the file.

new Manager doesn't really mean anything. There is no such thing as a manager (well, not in our code base :) ). However PluginManager definitely means something.

new Type\Manager does :)

Why would there be subclasses?

Eg Settings and Storage is a class that we have quite often. There could be eg Settings\Storage etc. Eg: - core/Measurable/MeasurableSetting.php - core/Measurable/MeasurableSettings.php - core/Measurable/MeasurableSettings/MeasurableSettingsStorage.php

vs - core/Measurable/Setting.php - core/Measurable/Settings.php - core/Measurable/Settings/Storage.php

I don't have a very strong opinion here but wanna clarify things and come to one solution that we can use in most of the cases across the code bases. Maybe we can get some more feedback from other devs? Maybe there are some blog posts about this that could be interesting?

@mattab commented on August 13th 2015

This RFC didn't lead anywhere, so we're closing it

This issue was closed on August 13th 2015
Powered by GitHub Issue Mirror