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

Add built-in support for auto-traversing Assembly trees #468

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

itsthejb
Copy link

Hi everyone,

Having used Swinject for many years, particularly the Assembler features, I was suddenly struck by the fact that the current implementation is "missing a trick".

In our rather large project, I/we have tried to make use of the loaded(...) hooks, but were surprised that they didn't behave as expected.

The approach we have is basically a big tree of Assembly. Assemblies tend to have children internally, and up until now we supported this by doing something like:

struct ParentAssembly: Assembly {

    let child1 = FooAssembly()
    let child2 = BarAssembly()

    func assemble(container: Container) {
        child1.assemble(container: container)
        child2.assemble(container: container)
        ...
    }

This works fine, but since the loaded(...) hook is a feature of Assembler, it won't be called; only on the "flat" array of assemblies passed when the Assembler is created.

It seems that the "built-in" way to do this is to use Assembler parent/child relationships. However, this requires a reference to the parent Assembler, and felt rather unintuitive, so we took this "lighter-weight" approach.

However, it dawned on me: why doesn't Assembly/Assembler support expressing a hierarchy out-of-the-box?

By using the method provided, it is possible to express a full tree using a single Assembler and Container if desired (this is what we do). Then all the loaded(...) callbacks are also called as expected.

Since the default is a no-op, this doesn't change anything if the feature is unused.

Currently this works in a sort-of depth-first order, although not quite. It could also be done in a strict depth-first, but I figured this is probably not so important; on the other hand, it makes sense that node assemblies have loaded(...) called first, just that the order of the nodes probably doesn't matter much. This could of course be changed, but would require using a stack, as with normal depth-first.

Also, trees could contain cycles, so that could be added, but also starts feeling a little heavy-weight.

Anyway, this is an idea/concept. Please let me know what you think.

Thanks


struct NestedAssembly: Assembly {

typealias ArrayType = NSMutableArray
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The testing concept injects two (reference type) arrays to allow tracking the order of the assemble and load operations. Seems easiest to do with NSMutableArray. Just first thing that came to mind

Comment on lines +347 to +350
let d = NestedAssembly(name: "d", children: [])
let c = NestedAssembly(name: "c", children: [d])
let b = NestedAssembly(name: "b", children: [])
let a = NestedAssembly(name: "a", children: [b, c])
Copy link
Author

@itsthejb itsthejb Jan 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple tree:

A
| \
B C
   \
    D

Comment on lines +353 to +354
expect(assemble).to(equal(["b", "d", "c", "a"]))
expect(loaded).to(equal(["b", "d", "c", "a"]))
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not strictly depth-first, since B->D->C->A, but may be good enough

@wintermoon9
Copy link

I was also thinking the exact same thing. We do something very similar at my company. Would love to see this become a part of Swinject.

@mpdifran
Copy link
Member

Assemblies tend to have children internally

Do they? They're all registering to the same Container, so I'm not sure what you get from the hierarchy of Assemblies. You would be able to get the same behaviour if you flattened your Assemblies.

I'll typically do something like this to register large groups of assemblies:

let coreAssemblies: [Assembly] = [FirstAssembly(), SecondAssembly(), ...]

assembler.apply(assemblies: coreAssemblies)

@itsthejb
Copy link
Author

itsthejb commented Nov 5, 2021

@mpdifran This is certainly how we were using it in a very large project, with quite a big tree of dependencies. It was understood at the beginning that this wasn't harmful, since re-registering services (because a node appears more than once in the tree) would just overwrite the registration with the same thing. Proved to be not so straightforward, though!

So yes, since making this I've become more inclined to agree that there's no way around flattening the tree. I actually wondered about building some component that was an Assembly wrapper, but internally would flatten and unique the tree. Certainly do-able. No idea if that would be useful to anyone else, though!

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

Successfully merging this pull request may close these issues.

None yet

3 participants