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

[BUG]: Phalcon\Di::setRaw() and getRaw() are misleading and/or incorrect #14555

Closed
scrnjakovic opened this issue Nov 24, 2019 · 1 comment
Closed
Assignees
Labels
breaks bc Functionality that breaks Backwards Compatibility bug A bug report status: medium Medium

Comments

@scrnjakovic
Copy link
Contributor

scrnjakovic commented Nov 24, 2019

So Di has these three methods (Zephir translated to PHP):

  • getRaw(string $name) : mixed which returns service definition, i.e. return $service->getDefinition()
  • setRaw(string $name, ServiceInterface $service) which sets a service, i.e. $this->services[$name] = $service
  • getService(string $name) : ServiceInterface which returns a service, i.e. return $this->services[$name]

Obviously, getRaw() and setRaw() do not match. By looking at the names, one would assume that ifgetRaw() returns just the definition, then setRaw() must set the definition of the service with the name provided as the first parameter, which is not the case. Instead, setRaw() actually sets the service, therefore it would make more sense to rename setRaw() to setService().

Furthermore, I think that name argument in setRaw() is redundant, given that it could be obtained from the service which is the second argument.

public function setRaw(string $name, ServiceInterface $service)
{
    ...
    $this->services[$name] = $service;
    ...
}

Should be

public function setService(ServiceInterface $service)
{
    ...
    $name = $service->getName();
    $this->services[$name] = $service;
    ...
}

It doesn't need to break BC:

/**
 * Must be properly documented here...
 */
public function setRaw(string $name, ServiceInterface $service)
{
    $this->setService($service);
}

public function setService(ServiceInterface $service)
{
    ...
    $name = $service->getName();
    $this->services[$name] = $service;
    ...
}

cc @niden @sergeyklay @Jurigag

@niden niden added 4.0 breaks bc Functionality that breaks Backwards Compatibility and removed Bug - Unverified labels Nov 29, 2019
@niden niden mentioned this issue Nov 29, 2019
5 tasks
niden added a commit that referenced this issue Nov 30, 2019
niden added a commit that referenced this issue Nov 30, 2019
niden added a commit that referenced this issue Nov 30, 2019
niden added a commit that referenced this issue Nov 30, 2019
@niden
Copy link
Member

niden commented Nov 30, 2019

Resolved in #14561

@niden niden closed this as completed Nov 30, 2019
@niden niden added bug A bug report status: medium Medium and removed Bug - Medium labels Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks bc Functionality that breaks Backwards Compatibility bug A bug report status: medium Medium
Projects
None yet
Development

No branches or pull requests

2 participants