A few observations

[sagikazarmark]

Here are a few observations:

• Application instance should clearly be a singleton instance. I couldn't find any reasons why a dev should be able to have multiple.
• We should get rid of the injection factory and leave injections to the dic. The new one will handle deps quite wisely.
• We should stop using the container as a service locator (looking at you, request stack). It should be injected into the application IMO.
• In general the application object should hold everything which is needed by the framework and everything should be retrieved from it if possible.

[WanWizard]

• The reason it isn't a singleton is that you should be able to define multiple applications within a single webserver instance. It is an additional level of abstraction, which can be used for multiple (separate) application components, like for example a frontend and a backend application. Note that the code to suppport using them isn't present yet, index.php just creates a single application instance now.
• The problem where was that object instantiation needs to happen from within the class, but you don't want to create a dependency with either the DiC or the required class. So this is not about deps, it is about a class requesting an instance of "something" without defining what that "something" is.
• I would like to see your suggestions for an alternative to create objects trees, so you can access a parent or child objects.
• I agree, I think for most services that is already the case.

[sagikazarmark]

1. In this case the application itself is still a singleton. You should not be able to fetch whichever application in an environment, only a specific one.
2. In most cases the injection factory simply gets an instance from the DiC. So I don't really get the point.
3. Have to think about that
4. Noop

[WanWizard]

• No, there can and will be multiple application objects simultaneously. This has not been implemented yet, but is required functionality
• Same as above. The code that is there is needed to make it work, it is not the final code. I'm not referring to the current codebase, but again, to the required functionallity.
• Ok
• Can you be more specific, maybe I misunderstood you.

[sagikazarmark]

1. Can you explain me this (with a possible use case)? It seems illogical to me (though it might make sense, I just don't get it)
2. IIRC we already talked about this InjectionFactory thing. At that time you said you didn't like it. Same as above: I don't see the benefit while it adds extra complexity (which might make sense again)
3. No more comments, just a joke (noop) ;)

I always have the feeling that the reason why fuel is not so popular is that there are some parts of it which are too complex. Then I always realise that there is a reason for that and makes fuel better than its competitors. That said, the complexity is still an issue for some people. If we could clarify things (even in the docs) than it would be awesome. You see, even me, a massive fueler (working with fuel EVERY SINGLE DAY) have some questions like these.

[WanWizard]

You can see it as an additional level of abstraction within the application. For example, all our applications consist of two parts: a frontend (one or more) and a backend (one or more). The are defined as separate applications, and either run in the same virtualhost, or on a different one, even on a different machine. This allows for instant scaling without a single change to the application. They may not be aware of each other at the PHP level, and communicate through an API using HMVC alike calls. This is all transparent for those using only one application. The application class is already prepared for this, there is no HMVC mechanism for it yet.

I totally agree with you on the factory uglyness. What I'm saying is that the target class (the class now using the factory) may not be tightly coupled to the DiC, and may also not be tightly coupled to the target class (the class it wants a new instance of). And this is the problem the factory currently solves (for the moment thru hardcoding, but obviously it has to use the DiC to request instances).

Complexity is never good. On the other hand, every toolbox contains tools that are more useful that others for a particular person. The challenge is that tools you don't use should be in your way. Like for example Theme in v1. If you don't need it, don't use it, and you won't know is there.

This also applies to the entire application discussion: it your apps only need a single Application object, why worry about the fact there may be more?

[sagikazarmark]

Damn, firefox crashed and my little novel went away. I am not going to rewrite the whole, but only the most important parts.

First of all: Injection Factory sucks. (Proof) It breaks the whole dependency injection thing. I still think it is a mistake to put bootstrapping logic into the application ctor. (The situation: the application uses the container to resolve resources which has dependencies which are not there because the Application injects them later. WTF?) This circular dependency thing makes me crazy.

Secondly: I made some code cleaning in the whole project, standardized dockblocks, removed some legacy code, fixed some issues, etc. It was a good experience because I got an overall view of the framework, but also showed me that we have much work left.

Last but not least: along with code cleaning I cleaned up a few repos as well (I mean project files, readme, etc). I am going to standardize these things a bit so we can be consistent about how repos look like. However you should take a look when you have some time. ;)

[WanWizard]

Agreed with the 'it sucks' part. 😆

The issue here was:

• We don't want classes to have a dependendy to the DiC itself
• We don't want classes to have any dependency to any other classes
• We need a solution in which a class can create new class instances at runtime without dependency

This means you can not:

• inject a DiC instance
• use new ClassName(); to create a new instance

And you're stuck. This was the only solution Frank could come up with at the moment, to handle the requirements, and fix a blocking issue. It was never meant to be a permanent solution (the InjectionFactory class violates the same rules), so if you have a better solution, I'm all 👂's.

[sagikazarmark]

We don't want classes to have a dependendy to the DiC itself

Why is that? Also, currently classes ARE resolved from the DiC. InjectionFactory uses it internally.

In this particular case (view manager) I don't see why it can't be resolved from the dic. Everything would be a lot more easier. And it doesn't mean the application cannot alter the state.

So I think I either don't understand or disagree with the arguments which led to this injection factory thing as a solution.

[WanWizard]

Perhaps I wasn't clear enough.

The InjectionFactory is NOT a solution, it is a workaround for a problem we had no solution for at the moment it was written. And it blocked the rest of the foundation package. I know the factory uses the DiC, that was the part we didn't have a solution for, and at least it meant the classes themselfs don't have a dependency on the DiC.

What we didn't want at the time was:

class Someclass
{
    public function __construct($dic)
    {
        $instance = $dic->resolve('SomeOtherClass', array('params'));
    }

because that makes Someclass dependent on the DiC. But you also can not inject an instance of SomeOtherClass, because at the time SomeClass is instantiated, you don't have the params you need to pass to the SomeOtherClass constructor yet. Frank wanted the option of be able to use a third-party DiC.

Now, if we say we don't have that requirement anymore, and define some interface any DiC most implement, we no longer need the factory but can have the class resolve itself. But then I still would like to decouple the class from the DiC, by using an abstraction layer.

namespace FuelPHP\Foundation;

class DiC
{
    public function  __construct($externalDiC)
    {
        $this->dic = $externalDiC;
    }

   public function resolve($class, $parms)
   {
        // use an external DiC method to get an instance of some sort
        return $this->dic->findAndInstantiate($class, $params);
}

Then we can inject an instance of this class in all FuelPHP classes requiring DiC access, and still have sufficient abstraction. If you want to use a different DiC, just write a new class that functionality does the same, but interfaces with the other DiC. No changes needed to FuelPHP classes then.

[sagikazarmark]

Well, I see some things now. (For the record: I strictly typehinted our implementation for now where a container is injected, we can change it back later if needed).

Actually I would like to see class instantiations moved out of the Application object. But I see now that the state is not always available, so we have to find the point in the application where we have the state (config, etc) available, but still outside of the Application constructor.

[sagikazarmark]

Ok, further thinking about it, my primary issue is (strongly tied to the code) that some of the initialization code is placed in the constructor, which means it won't be available in the dic until the constructor finishes, but in some cases we want to get it during the constructor is running.

So how about breaking things into two parts: create the application object, save it into the container and do initialization afterwards. I am not sure if it solves all of our problems, but does a few which causes me headache in the past few days.

[sagikazarmark]

@WanWizard what do you think about this? It would save us from a lot of DI bullshit.

$app = new Application;
$dic->add('application::'.$name);
$app->initialize();

Currently the initialization logic is full of circular dependencies: depending on application when it is not even in the dic, etc. This would make the app available in the container, however if someone tries to use a feature before it is initialized....that person deserves what he/she get. 😜

[WanWizard]

I am open for any improvement. 😄

However, there is reason for the current madness, so you have to be careful when stripping existing functionality. Most objects are tied to an application object. Take for example the ViewManager, if you need that, you need to be able to get the Application instance from the DiC first. Which is impossible in your example unless you know the name (which you usually don't). This is the reason the first app object is also stored as "application::main".

[sagikazarmark]

Right. But how does it related to the solution above? The difference is that the application is available in the DIC before initialization logic. Actually, some application dependencies require the application in the dic. The only thing we have to care about is to don't rely on something which is no available in the application object at that time. For example parsers in display require a view manager instance which is resolved by the dic. When parsers are requested from the dic, the application is already available in the dic, so as the view manager. Problem solved.

[sagikazarmark]

Related to injection factory thing: I like the way how event emitter does this: it has a trait which exposes a getter and a setter for an event emitter. If no custom implementation is set, a default implementation is instantiated (and set) upon first usage.