Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Access service locator #4

Merged
merged 5 commits into from
Sep 10, 2014

Conversation

codeliner
Copy link
Contributor

Hey,

thank you for this great module. I need the possiblity to access the ServiceLocator in my migrations, cause I'm using an EventStore for my write model and ZF2 Tablegatway for my read model. I need access to my EventStore and this can be achieved by requsting it from the ServiceLocator. I think this can be useful in other scenarios, too. What do you think?

@vgarvardt
Copy link
Owner

It's fine about functionality, but I have some notes on the code:

  • .gitignore must not be here
  • as Migration now has ServiceLocator it must implement ServiceLocatorAwareInterface
  • the easiest way to implement ServiceLocatorAwareInterface is to use ServiceLocatorAwareTrait, but module should work under php 5.3, so you'd better copy that trait into Migration (it's about using same names across the project)

@vgarvardt
Copy link
Owner

And one more thing - please update README to reflect your change, include note on possibility to implement ServiceLocatorAwareInterface for migrations. Thank you.

@texdc
Copy link

texdc commented Sep 9, 2014

I need the possiblity to access the ServiceLocator in my migrations

Not everyone needs a ServiceLocator. Instead, extend the class as LocatorMigration and inject it in the constructor.

@codeliner
Copy link
Contributor Author

@texdc Not everyone gets the ServiceLocator. You need to implement the interface in your migration class to get access to it!

Extending the Migration class is not that easy, cause it is not injected in the controller. But anyway I think with the current implementation both scenarios are perfectly supported.

@codeliner
Copy link
Contributor Author

@vgarvardt Sorry for the .gitignore. You are absolutly right with using consistent naming. I've changed my implementation. Hope that this version finds the way in your repo :-). Btw. do you plan to release a v0.1.0? Would be great to have a version that can be referenced in composer requirement.

@texdc
Copy link

texdc commented Sep 9, 2014

@codeliner You're missing the point. Adding a ServiceLocator instead of the specific dependency you need is poor form. It's a heavy-weight dependency that is not necessary.

@codeliner
Copy link
Contributor Author

@texdc I would totally agree with you when we would speak about application code. But in this case I think the module should be as flexible as possible. Why not give a user of the module the possibilty to access the service locator in a migration class? It can be achieved with a few lines of code and with standard techniques provided by ZF2. The module stays lean but offers a lot more usage options than just writing sql to up- or downgrade your database.

It's a heavy-weight dependency that is not necessary

Every controller in the MVC Stack has this "heavy-weight dependency". Why not also provide a link to it in a migration? It's just a pointer to an already existing object.

@vgarvardt It's your choice :-). Under which license is your library available? If you decide to not include my pull request, can I fork your module and adjust it to meet my needs?

vgarvardt added a commit that referenced this pull request Sep 10, 2014
@vgarvardt vgarvardt merged commit 305484c into vgarvardt:master Sep 10, 2014
@codeliner
Copy link
Contributor Author

👍

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

Successfully merging this pull request may close these issues.

3 participants