I have a general question. Why do we need macros for injector? What is the critical improvement it gives us that we should go with the macro and preprocessor logic over simple C++ syntax?

What if we had a class, let's say Application (like UnitEInjector now) which in its constructor initializes each component in the same order we have now and in the destructor invokes the delete on each object in the correct order?

pseudo code:

class Application {
public:

Application() {
  m_state_register = new StateRegisterImpl();
  m_state_processor = new StateRegisterImpl(m_state_register);
  // ...
}

~Application() {
  delete m_state_processor;
  delete m_state_register;
}

StateRegister GetStateRegister() { return m_state_register; }
StateProcessor GetStateProcessor() { return m_state_processor; }

private:
  StateRegister *m_state_register;
  StateProcessor *m_state_register;
}

I see that with macro we don't need to write initializer, deinitializer and getter methods so it's an improvement. But having a bit of boilerplate (we don't create new components often, and at the code review we can check that they are initialized properly), we would have a simple C++ syntax.

Then we don't need templated GetCompoment function, we would just call g_application.GetStateProcessor() for instance. Then we don't need Dependency<...> alias, we will direcrly use * or unique_ptr or whatever the reference is.

Can you please open an issue about this if you feel strongly about this instead of polluting pull requests with your agenda?

Thank you for the thumbs up, despite my rude tone. I copied the comment to an issue and commented there: #723

I also started work on fixing #618. I thought to come up with complete replacement of NAMEs in the injector, but let's start from GetComponent. You can find my approach to fix this issue in #726.

On the one hand, I like this because it makes easier to reason about types, not only to us, but also to the IDEs & static analysis tools. On the other, I'm unsure about what other benefits it provides over the current code.

@castarco is that a thumbs up or a thumbs down or a I don't know, I'm undecided?

I also started work on fixing #618. I thought to come up with complete replacement of NAMEs in the injector, but let's start from GetComponent. You can find my approach to fix this issue in #726.

I looked at #726. What I like about my version better is that if you do GetComponent<X> where X is not actually defined in the injector, it will not compile. In #726 it throws this UnknownComponentError. I strictly like things which error out on compile time already better.

I looked at #726. What I like about my version better is that if you do GetComponent where X is not actually defined in the injector, it will not compile. In #726 it throws this UnknownComponentError. I strictly like things which error out on compile time already better.

Yes, until we have runtime-only container for holding components, it's a runtime check. Basically, what I don't like in this PR is a nullptr-casting magic, especially on the API level (see gathering Settings in the walletextention).

@castarco is that a thumbs up or a thumbs down or a I don't know, I'm undecided?

@scravy mostly a thumbs up, I was just curious about what reasons are behind this change beyond what I could grasp.

especially on the API level

That API is not exactly public. I can make it private and befriend GetComponent though to make that clear.

I don't like in this PR is a nullptr-casting magic

Happens all the time, you weren't exactly bothered by this in std::enable_if<...static_cast<T *>(nullptr)->ToString()>>. It's an internal trick to select the right method using overloading. I would have used a template specialization but these are not allowed inside classes for historical reasons.

Yes, until we have runtime-only container for holding components

I don't get that and I'm not sure I want that.

Another thing I like better in here is that getting a component is constant, whereas in #726 needs to look it up in a tree map.

@castarco is that a thumbs up or a thumbs down or a I don't know, I'm undecided?

@scravy mostly a thumbs up, I was just curious about what reasons are behind this change beyond what I could grasp.

Thanks :-) – There's a detailed discussion also about this whole thing in #723, feel free to chime in there!

That API is not exactly public. I can make it private and befriend GetComponent though to make that clear.

Happens all the time, you weren't exactly bothered by this in std::enable_if<...static_cast<T *>(nullptr)->ToString()>>. It's an internal trick to select the right method using overloading. I would have used a template specialization but these are not allowed inside classes for historical reasons.

I complained about the first version where Get(T*) was a public API. Now it looks great.