Integrate a Logger-Class with a namespace, maybe PSR3-Logger?

[b3nl]

You use a "Logger"-Class without any namespace. This is very bad style for an external library and will break many projects (as it does with mine).

[rudibatt]

The Logger is from log4php; we haven't change anything at that source code, just copied it into the project. Unfortunately Log4php is still not using namespaces, so a update wouldn't fix it.

The library itself uses the FACTFinder\Util\LoggerInterface - always with namespace notation. Log4php is included into the project by default using the FACTFinder\Util\Log4PhpLogger implementation. If you have a different logger, you can create a similar LoggerInterface implementation and simply delete log4php.

[b3nl]

Hi @rudibatt why do you close the ticket? Is a plan, to deliver a more robust library such a bad idea?

[rudibatt]

I closed it, because there is no solution here. The log4php logger can't be fixed as we won't change that code and if log4php shouldn't be used, this can be replaced in a project.

...or did I miss understand it wrong and you suggest to replace log4php per default?

[rudibatt]

I just checked PSR3-Logger and this looks very similar to our LoggerInterface and as far as I could see it does not provide a default logger; it's also just that LoggerInterface. So I don't get this issue..

[b3nl]

Thx for your reply.
Our real problem behind this, is: that you break the typical depency injection style with your logger class. For example FACTFinder\Adapter\Search::__construct gets type-hinted injections, but the logger will be provided by class name and loaded internally.
(This behaviour leads to even more problems. If you try to load this factfincder lib with i.e. the symfony dependency injection component, you need to take additional steps.)

The missing namespaces makes matters worse. But i did not want to discuss DI styles till now - you can not be responsible for every possible DI-Framework there is - but using not secure namespaces is a responsibility of you.

PS: PSR3 was a suggestion for the refactoring of the used logger mechanics

[rudibatt]

Thanks for your explanation.
Yes, the logger classname injection is ugly and was only done like that, because it needs the name of the logging class (so some kind of cyclomatic dependency). Would a replacement of this kind of injection solve the problem?

[b3nl]

+1
Maybe you could work with an optional setter-Injection for a logger. And an optional getter could check the logger property internally and lazy loads the default logger per default, if there was not one injected before.

[rudibatt]

yeah ..or the naming of logger can be done afterwards or not at all anymore. This is how it would work with the PSR3-Logger.

@abognetti: are you still developing this lib? Could you do one of the proposed solutions?

[b3nl]

I could try my best aswell. (without psr3 firstly)

[b3nl]

Is there something new here? Or should i try my best?

[rudibatt]

Seems like do not have a maintainer here anymore... :(
So I guess you can go for it.