Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Circular Referencies (Dependencies) #139

Open
fljot opened this issue Jun 5, 2013 · 4 comments
Open

Circular Referencies (Dependencies) #139

fljot opened this issue Jun 5, 2013 · 4 comments

Comments

@fljot
Copy link

fljot commented Jun 5, 2013

I do realize it's a pure SwiftSuspenders issue, but since Robotlegs v2 is about to be released and SwiftSuspenders is some sense the base of RL and RL is definitely more popular project out of two...

It seems like SwiftSuspenders still can't handle circular referencies as mentioned before here tschneidereit/SwiftSuspenders#11.
I tried with this commit https://github.com/tschneidereit/SwiftSuspenders/tree/4205e4e4376f6769834399bc8159dfa62d04860e, the test code look as follows:

package
{

import flash.display.Sprite;

import org.swiftsuspenders.Injector;


public class SwiftSuspendersCircularDependenciesTest extends Sprite
{
    public function SwiftSuspendersCircularDependenciesTest()
    {
        const injector:Injector = new Injector();
        injector.map(Foo).asSingleton();
        injector.map(Bar).asSingleton();

        injector.getOrCreateNewInstance(Foo);
    }
}
}
package
{
public class Foo
{
    [Inject]
    public var bar:Bar;

    public function Foo()
    {
        trace("Foo created");
    }

    [PostConstruct]
    public function onInjected():void
    {
        trace("bar in Foo is", this.bar);
    }
}
}
package
{
public class Bar
{
    [Inject]
    public var foo:Foo;

    public function Bar()
    {
        trace("Bar created");
    }

    [PostConstruct]
    public function onInjected():void
    {
        trace("foo in Bar is", this.foo);
    }
}
}

Output is:

[trace] Foo created
[trace] Bar created
[trace] Foo created
[trace] Bar created
...

with a following stack overflow.

@creynders
Copy link
Member

Hmmm.... circular dependencies should really be avoided as a best practice. I don't know whether it's actually a good idea to try and support them.
Maybe an error should be thrown though. (No idea if that's feasible or how much work would be involved though)

@bsideup
Copy link

bsideup commented Jun 5, 2013

You are wrong. One of the main concepts of IoC is to avoid of dependency hell and helps to rule-em-all:)
It is usual scenario when one service is referencing another, for example we have two processors (proc1 and proc2) of data, and both of them ask each other about type of data:

class ConcreteMediatorHelper {
    [Inject] public var mediator : SomeMediator;

    override public function help()  {
        mediator.showHelpScreen();
    }
}

class SomeMediator extends Mediator {
    [Inject] public var concreteMediatorHelper : ConcreteMediatorHelper;

    public function doSomeStuff() {
        concreteMediatorHelper.help();
    }

this is a simplified example of issue in our project. And it really should be fixed, just look in Java Spring IoC, there are cyclic references support from the begining AFAIK.

@darscan
Copy link
Member

darscan commented Jun 5, 2013

Hi @fljot and @bsideup , thanks for the input. Looking at the original Swiftsuspenders issue, I worry that Till couldn't think of a nice way to integrate this. Please feel free to fork the code and submit a patch if you have ideas for a concrete solution.

@creynders
Copy link
Member

@bsideup

just look in Java Spring IoC, there are cyclic references support from the begining AFAIK.

That doesn't mean it's good practice though. Circular dependencies are a serious code smell, and IMO we shouldn't be encouraging bad habits. I can only imagine benefits for circular dependencies in data structures, but never for actual components. And in the case of data structures it's the responsibility of the structure itself (or its composer) to fullfil and maintain the dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants