bindInstance with copy about fruit HOT 13 CLOSED

Hi, the problem with that is that a NormalizedComponent can be used to create multiple injectors, so Fruit can't move ownership of the instance from a component into any single injector.
If you're ok keeping memory for that object allocated until the process ends, you can do something like:

bindInstance(* new MyObject(...))

If this is done outside a NormalizedComponent and you're creating multiple injectors it would do multiple allocations but IIUC that's not your case (?).

Or otherwise you can save that object in a static variable and pass the reference to that:

static MyObject x = ...;
bindInstance(x);

If neither of these works for you, can you explain why so that I might suggest sth else?
You shouldn't have to explicitly keep or pass around other objects together with the injector instance.

from fruit.

If neither of these works for you, can you explain why so that I might suggest sth else?
You shouldn't have to explicitly keep or pass around other objects together with the injector instance.

They technically work, but they are against my own best practices of minimizing global variables. In fact, the whole reason for me to adopt a DI framework is to avoid global variables but having their convenience of not needing to pass data through multiple layers. If I had to resort to global variables after all, I wouldn't want to complicate my life with DI. I would just save everything globally.

If you're ok keeping memory for that object allocated until the process ends, you can do something like:

This has the same problem as global variables, in that the callee needs to know how the caller intends to use it, and force the whole program to be strongly coupled together. Which is the exact inverse of what dependency injection is trying to achieve---loose coupling between components.

Hi, the problem with that is that a NormalizedComponent can be used to create multiple injectors, so Fruit can't move ownership of the instance from a component into any single injector.

Then I have two suggestions:

1. a bindInstanceCopy that requires T is copyable and only injected as value, never as references or pointers. Then each injector can have its own copy, and users cannot tell apart if they are the same instance.
2. a bindSharedInstance taking a std::shared_ptr<T>. All injectors from the same NormalizedComponent are referring to the same T, and they are all keeping T alive through the reference count.

from fruit.

I see the main benefit of DI as being able to reuse common modules in multiple production systems and to construct test systems that have arbitrary sets of production modules mixed with some test-only ones.

Using a local static var doesn't necessarily invalidate these.
That object will be constructed only if needed and users of it don't need to declare direct deps on it so that top-level code can e.g. decide to swap in a different module in a unit test and then the static local var would never be initialized.

I think the variant you mentioned of a bindInstanceCopy would work, though complicating things a bit (now Fruit's components would need to keep ownership of these while ATM only fruit injectors own user objects).

Can you expand on how the two suggestions above result in tighter coupling? Maybe with an example?
ATM it's not clear to me if this extra api and complications in the fruit impl are really worth adding.

Thanks

from fruit.

That object will be constructed only if needed and users of it don't need to declare direct deps on it so that top-level code can e.g. decide to swap in a different module in a unit test and then the static local var would never be initialized.

But that component function will mandate that its caller is top level and can call it only once. That's the coupling I'm talking about.

from fruit.

from fruit.

If it is modified so copies are needed, the component function with the
static var could bind that with an annotation type that is not visible
elsewhere and bind a non-annotated version of the same type with a provider
lambda that copies it.
Then any code injecting it would get 1 copy per injector that can be
modified at will.

Then this is imposing a requirement on the callers that they must be serialized with respect to each other. The bane of globally mutable state--multithreading.

Of course, we can add mutex here, but

• It's quite counterintuitive as the locking happens outside the provider when the global state is assigned to, and unlocking happens inside the provider, after the global state is copied.
• It's much more complex than bindInstanceCopy.

Btw another alternative to the static var approach is a provider lambda
that constructs the object there.

I do use a lot of providers. But providers need a starting point, a terminal node, and that can only be provided by bindInstance (or static var, which I am against) in my case.

Anyway, all of your solutions, and my current one (keeping the references alive manually), relies on global reasoning. That is to say, every part of the program needs to know what other parts are doing in order for the whole program to be correct.

What I prefer to have is a model requiring only local reasoning. Each part only needs to reason about its own correctness, and the program will be correct regardless how all parts are wired together.

from fruit.

from fruit.

So the caller should be able to modify the static var to accommodate different states, correct? Then what happens if another caller also modifies this static var, and before the first caller copies this value in the provider function?

I don't see how this solution relies on global reasoning, concurrent
copyability is a local constraint.

Yes, a mutex will make this a local constraint. But that is way more complicated than bindInstanceCopy. You believe implementing bindInstanceCopy will make the fruit internals more complicated, but a framework is supposed to shield its users from complexity, not shifting complexity to them.

from fruit.

To be more precise, let's say we have this class

struct MyClass {
  INJECT(MyClass(ANNOTATED(tMyParam, int) myParam));
};

fruit::Component<MyClass> getMyComponent(int param);

I'm assuming that your approach would be like this

fruit::Component<MyClass> getMyComponent(int param) {
  static int savedMyParam;
  savedMyParam = param;
  return fruit::createComponent().registerProvider([]() {return new MyClass(savedMyParam);});
}

Or this

fruit::Component<MyClass> getMyComponent(int param) {
  static int savedMyParam;
  savedMyParam = param;
  return fruit::createComponent().registerProvider<fruit::Annotated<tMyParam, int>()>([]() {return savedMyParam;});
}

In both cases, if two threads call this method concurrently with different values, then when the provider lambda is called by the Injector construction, its value may have been changed by the other thread. To solve this, we can add a mutex here.

fruit::Component<MyClass> getMyComponent(int param) {
  static int savedMyParam;
  static std::mutex mutex;
  mutex.lock();
  savedMyParam = param;
  return fruit::createComponent().registerProvider<fruit::Annotated<tMyParam, int>()>([]() {
    int localValue = savedMyParam;
    mutex.unlock();
    return localValue;
});
}

Now this approaches have multiple problems:

1. It is complex.
2. It is counterintuitive (the mutex locking and unlocking happen in different scopes).
3. It reduces parallelism by introducing a contention point.
4. It creates a static storage never to be released.

Now, if my proposal is accepted, it will be like

fruit::Component<MyClass> getMyComponent(int param) {
  return fruit::createComponent().bindInstanceCopy<fruit::Annotated<tMyParam, int>>(param);
}

The simplicity and elegance is self evident. And it has no chance of any kind of mistakes, memory safety or thread safety.

from fruit.

from fruit.

• top-level, where you create the injector. In which case you can have a
  local var for that value in the same scope where the injector instance is.

And then I have to bundle the variable with the injector together, into a "super" injector, to ensure that they are both alive at the same time. This is what I am currently doing, but it is clunky, and you seem to agree with me before

You shouldn't have to explicitly keep or pass around other objects together with the injector instance.

from fruit.

And then I have to bundle the variable with the injector together, into a "super" injector, to ensure that they are both alive at the same time. This is what I am currently doing, but it is clunky, and you seem to agree with me before

I agree that having to create a bundle (that I take to mean like a struct with the injector and some other objects) is odd.
Could you provide some example code for the injector creation/use part?

I'd expect the Injector object to be a local var in most cases (whether it's in main or a main-adjacent function for process-wide injectors, or in the toplevel function/method that implements an RPC handler, or in a test case, or similar).
And in that case bundling would not be necessary, you'd just have some other local vars, possibly in the same function.
(e.g. see https://github.com/google/fruit/wiki/tutorial:-server)

And the injector instance would not be passed around, it'd only be used to inject the toplevel object (or a few) and then control would pass to their methods (that have access to specific injected objects so don't need to take the injector object as param, nor get ownership).

Can you explain more why bundling is needed in your case ATM?
I.e., does the use case that you have in mind involve allocating injector objects on the heap, or transferring ownership of the injector objects, or storing multiple injectors in a collection, or ...?

from fruit.

Your approach works but it is still not to my taste. But that is probably not a strong argument for you to redesign fruit.

from fruit.