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

Using extended classes in project #914

Closed
vitek-rostislav opened this issue Mar 27, 2019 · 4 comments · Fixed by #1344
Closed

Using extended classes in project #914

vitek-rostislav opened this issue Mar 27, 2019 · 4 comments · Fixed by #1344
Labels
Help wanted We need your help or opinion how to resolve this

Comments

@vitek-rostislav
Copy link
Contributor

vitek-rostislav commented Mar 27, 2019

What is happening

Model situation:
In my project, I extend ArticleFacade, add there new public myNewFunction(), and configure my new service as an alias for the original one in services.yml, something like this:

Shopsys\FrameworkBundle\Model\Article\ArticleFacade: '@Shopsys\ShopBundle\Model\Article\ArticleFacade'

Then, I want to use ArticleFacade::myNewFunction() in Controller\Front\ArticleController that has a dependency on ArticleFacade from the core (ie. FrameworkBundle).

At the moment, I am able to use the new function thanks to the DI container configuration, however, the IDE is not aware of it as the dependency is still typehinted and annotated as the original ArticleFacade from the core, and therefore I am not able to click on the function myNewFunction and navigate to the proper class. This might be very confusing from a long-term maintainability point of view.

Possible solutions

  • solution 1 - I can just overwrite the property annotation:
/**
- * @var \Shopsys\FrameworkBundle\Model\Article\ArticleFacade
+ * @var \Shopsys\ShopBundle\Model\Article\ArticleFacade
 */
private $articleFacade;
  • solution 2 - I can change the dependency completely so the Controller is dependent on my own ArticleFacade (ie. the descendant implemented by me in ShopBundle) indeed:
- public function __construct(\Shopsys\FrameworkBundle\Model\Article\ArticleFacade $articleFacade)
+ public function __construct(\Shopsys\ShopBundle\Model\Article\ArticleFacade $articleFacade)
{
    $this->articleFacade = $articleFacade;
}

Personally, I prefer the solution 2. I am thinking about implementation of some fixer, that would automatically change all the dependencies in project-base in such a case, maybe it could add the entry to services.yml as well. Basically, if you extended a class, the fixer would ensure that it is used everywhere in your project instead of the parent class.

More on that, we should cover the situation when I extend some class that is dependent on a class that I have already extended:

  • ShopBundle\ArticleFacade extends FrameworkBundle\ArticleFacade
  • FrameworkBundle\Admin\ArticleController is dependent on FrameworkBundle\ArticleFacade - everything ok so far
  • ShopBundle\Admin\ArticleController extends FrameworkBundle\Admin\ArticleController - now I should probably ovewrite the dependency on FrameworkBundle\ArticleFacade and start using my ShopBundle\ArticleFacade

I would like to validate the fixer idea so I am asking here for your opinions or other suggestions.

Expected result

There is a consensus on the proper usage of extended classes in project.

@vitek-rostislav vitek-rostislav added the Help wanted We need your help or opinion how to resolve this label Mar 27, 2019
@jDolba
Copy link
Contributor

jDolba commented Mar 27, 2019

Is it possible to provide more related articles, docs, etc. covering your mind map about planned "Service extension-ability"? I have found articles and docs only about entity related extensions, I am not sure if it is same

Provided examples are too simple from my point of view and therefore little bit misleading

for example I want to use some third party "module" (or package), which also extends ArticleFacade with method extendedModuleFunction() and I want to add my new function/extension called myDreamArticleFunction() on top of this extension and now I want to use in my controller both functions

  • is it even expected to support usage like this?
  • with this example, you can not use your preferred solution by specifying type in constructor (or can you?)
  • phpdoc hint with both class names does not seem as good idea either

if it is only about IDE support and autocomplete; I would suggest to have a look at
.phpstorm.meta.php to help IDE to understand how extension works in SSFW

@stale
Copy link

stale bot commented Jul 25, 2019

This issue has been automatically marked as stale because there was no activity within the last 4 months (and it is quite a long time). It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale This issue have not been touched for some time and if it doesn't change, it will be closed soon. label Jul 25, 2019
@pk16011990
Copy link
Member

up

@stale stale bot removed the Stale This issue have not been touched for some time and if it doesn't change, it will be closed soon. label Jul 26, 2019
@vitek-rostislav
Copy link
Contributor Author

Hi, @simara-svatopluk is already analyzing possible solutions for this 😉

@vitek-rostislav vitek-rostislav added the Status: in implementation Resolving of this issue is currently in progress label Jul 26, 2019
@vitek-rostislav vitek-rostislav removed the Status: in implementation Resolving of this issue is currently in progress label Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help wanted We need your help or opinion how to resolve this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants