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

Feature: Return dependency as const when needed #110

Open
abeimler opened this issue Jan 25, 2023 · 2 comments
Open

Feature: Return dependency as const when needed #110

abeimler opened this issue Jan 25, 2023 · 2 comments

Comments

@abeimler
Copy link

abeimler commented Jan 25, 2023

Is your feature request related to a problem? Please describe.
I'm trying to make a (const) getter with return m_container.service<di::RegistryService>(), but I get an Error: error: no matching function for call to ‘kgr::container::service<di::RegistryService>() const’

I also try something like this: return m_container.service<const di::RegistryService>()

Describe the solution you'd like
make a kgr::container::service() const method

Describe alternatives you've considered
Right now I'm trying this as a workaround: return std::as_const(m_container.service<di::RegistryService>())

Additional context

  [[nodiscard]] engine::Registry& registry() {
    return m_container.service<di::RegistryService>();
  }
  [[nodiscard]] const engine::Registry& registry() const {
    return m_container.service<di::RegistryService>();
  }

  [[nodiscard]] float getFrameTime() const {
    if (registry().ctx().contains<const GameContext>()) { // registry() needs to be const
      const auto& game_ctx = registry().ctx().get<const GameContext>();
      // ... 
    }
    return 0;
  }
@gracicot
Copy link
Owner

A const service function would only be possible with supplied service and abstract service that has no default. Any other service could mutate the container to add itself to the list of single service. I'm not sure how I feel overloading the service function that would work only with some service types.

If I enable const service function for all services, I would have no other choice but to throw if a service is not found. This is basically the same problem as with the std::map operator[] function.

In the next version in the 4.x.y series I could add some kind of find function in the container that would return either null or the instance of the service, but I'm not sure when that would be as I'm busy with the 5.x.y series. A MR would certainly help.


There are some workaround possible in the meantime, but none are perfect.

The first one is that if you know in advance that the service is already there, you could just const cast away the const

[[nodiscard]] const engine::Registry& registry() const {
    if (m_container.contains<di::RegistryService>()) {
        return const_cast<kgr::container&>(m_container).service<di::RegistryService>();
    } else {
        throw something;
    }
}

If you don't want to const cast, you can fork the container then call the service function on the fork. If there is single services instantiated by the service call, it's going to be dropped with the temporary. If the service is already in the container there would be no problem.

[[nodiscard]] const engine::Registry& registry() const {
    if (m_container.contains<di::RegistryService>()) {
        return m_container.fork().service<di::RegistryService>();
    } else {
        throw something;
    }
}

The only problem is that if you end up creating services on the fork, you'll get nasty lifetime problems since the fork died. Also forking allocates (a little bit)

There are other workaround but those would ask to change your structure, which you may avoid. I'm writing those so also all future readers would know those solution exist.

Another way would be to just send it as parameter to getFrameTime instead of that function calling the container directly:

  [[nodiscard]] float getFrameTime(engine::Registry const& game_ctx) const {
      // ... 
  }

// elsewhere, call it with the container:

// have a mutable container here, then call the const function
container.invoke([](engine::Registry const& game_ctx) {
    return getFrameTime(game_ctx);
});

Or just contain the engine::Registry& object in the type that has the getFrameTime member function if it's indeed a member.

Let me know if this helps!

@abeimler
Copy link
Author

Also thought about const_cast, but that's just ugly.

I now just added an engine::Registry& m_registry as member and init it in the ctor

  Game() : m_registry{m_container.service<di::GameRegistryService>()},
           m_dispatcher{m_container.service<di::GameDispatcherService>()},
           ... {}

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

No branches or pull requests

2 participants