Dependency Injection / Code Decoupling

[shadowhand]

Right now the usage of static classes as factories makes it really had to extend the query builder on a per-project basis. A more decoupled approach that would allow for complete dependency injection, would be highly desirable.

Fundamentally, very little needs to change. Primary changes required are:

• using factories to create instances of all classes
• converting static classes to instances
• use some kind of service locator or class map

[shadowhand]

More careful analysis is needed to clearly identify where injections need to happen. If anyone has started research on this, please post your notes here so that effort is not duplicated.

[nilportugues]

I agree factores work nicely, but can be done better.

I was actually thinking of using class maps in this case to reduce memory usage, as no use-statements would be required for starters until the class is actually called upon and an instance is built.

I've been using this approach on other packages successfully and really pays off (eg: https://github.com/nilportugues/validator)

[shadowhand]

The primary problem I see with class mapping is that it often results in a loss of interface type checks. Maybe I've just never seen it implemented properly... even the Validator example is confusing... I'm not sure where $classMap is actually being invoked.

[shadowhand]

Closed? Has this been resolved?

[nilportugues]

Yeah, I rewrote the GenericBuilder in a $classMap fashion. Also Scrutinizer-CI gave the library a better grade.

Changes:

• https://github.com/nilportugues/sql-query-builder/blob/master/src/Builder/GenericBuilder.php#L61
• https://github.com/nilportugues/sql-query-builder/blob/master/src/Builder/GenericBuilder.php#L220

I checked everything else, and there's not much it can be done, I believe... perhaps grouping the other factories together and pull out line 61 to an Abstract Factory.

[shadowhand]

And how would someone inject a modification into that classmap? I don't see a setter function, and the class variable is private, so even if someone extended the class, they would be unable to overload the property...

[nilportugues]

Being core operations I don't intend to make the classMap dynamic...

Tell me about your approach, or a case it would come in handy.

[shadowhand]

@nilopc private variables are effectively saying "i am the smartest guy in the room, no one could ever possibly improve this or use it differently" which every programmer knows is a bogus statement. By using private (as opposed to protected), no one can ever extend the code to do something differently. The entire point of Dependency Inversion is to assist satisfying the Open/Closed Principle. Preventing extension directly violates SOLID programming and makes the software less usable.

[shadowhand]

All of which is a very roundabout way of saying that Query Builder has a long way to go to become extensible. I've not had any time to work on it, but I should have some time over Christmas break to refactor things into a more decoupled library. :)

[nilportugues]

I see your point now!