Skip to content

Conversation

@jkrzefski
Copy link
Contributor

Imagine you wanted to have a service with a constructor parameter of a string like this:

class UserController
{
    public function __construct(string $hostname)
    {
        // ...
    }
}

Prior to this change, you would have to do this.

$container = new FrameworkX\Container([
    UserController::class => function (string $hostname): UserController {
        return new UserController($hostname);
    },
    'hostname' => 'Some string value',
]);

After this change, you can do this:

$container = new FrameworkX\Container([
    'hostname' => 'Some string value',
]);

I am still relatively new to this framework, so I don't know whether this will break some anything.

@SimonFrings
Copy link
Contributor

Hey @jkrzefski, thanks for bringing this up 👍

This is something we also thought about when implementing the support for variables in container configuration for factory functions (#178, #179, #180, #182, #183, #184), we actually took the more difficult route on purpose. What you're suggesting is definitely a much easier way, but we are unsure what the consequences are if we start supporting this (name collisions, unwanted behavior, etc.).

It might be worth opening a discussion on this topic first and see what needs to be worked out before this new feature can be deployed. What do you think about this?

@jkrzefski
Copy link
Contributor Author

@SimonFrings That you for your feedback. (And sorry for my late response 😥)

I opened a discussion about this topic. #192

@clue clue added new feature New feature or request BC break labels Apr 7, 2023
@SimonFrings
Copy link
Contributor

@jkrzefski I think we can close this ticket here for now as we have the ongoing discussion in #192. Let's revisit this once we figured out the consequences and have an idea what different approaches can look like 👍

@SimonFrings SimonFrings closed this Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BC break new feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants