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

Ability to resolve sagas via container #22

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Ability to resolve sagas via container #22

wants to merge 7 commits into from

Conversation

aleksandrov
Copy link

Hi,

Here is my changes to make possible to resolve saga instances via container.
This patch fixes #20 issue in CommonComain.

@StasPerekrestov
Copy link

Could you please describe some use-cases when you need to use this feature.
Thanks.

zxsanny and others added 2 commits April 25, 2013 13:36
Support applying IList<Event> in AggregateBase. It is useful in case of EventStore upconverter returns list of events instead of single event.
@aleksandrov
Copy link
Author

It is useful when you need to inject some dependencies into Saga instance. I know, it is possible to use Service Locator for that, but Constructor Injection is more correct to do that.

@gregoryyoung
Copy link

what exactly would you be doing with an injected item in a saga? It is not generally advisable to be doing such things. You should be sending a message to something that does it.

@aleksandrov
Copy link
Author

Yes, you are generally right. But in some cases external services can help Saga to make a decision which message to send.

Anyway, it is good to have such possibility (Aggregates actually has) and let developers decide to use it or not.

@gregoryyoung
Copy link

I would have to say its a bad idea because developers will use it. I can not (trying hard) come up with a single use case where I would actually want this. What I can come up with is 10000 use cases where it would be misused. When dealing with sagas you should be dealing with messages. If you need to access some service to get information to make a decision you should be messaging it not injecting it and calling it.

When people talk about process managers (these are not really sagas) one of the most important ideas is that they are "fancy message routers". If you start putting dependencies etc into you will likely make a large mess.

Greg

@aleksandrov
Copy link
Author

Sagas still can be used in wrong way by using Service Locator. Using Activator.CreateInstance() to create Saga instances does not solve that problem. Basic idea is to have consistent way to create instances inside CommonDomain.

Dmitry

@gregoryyoung
Copy link

They can also be in the wrong way by opening up a windows form, asking an admin to fill in some information :)

Normally I would say "design to help people understand". When people see "ooh I can just pass dependencies" they think its ok. For me I would make it so you couldn't do any of this at all (impossible) at a language level and my previous dsl for managers actually did this (forced you to a language where you could olnly send/receive messages, any code you put in was dynamically mapped into services and messaged!)

@antonsamarsky
Copy link

IMHO, is such a way you will actually not making any languages restrictions, just inconvenience. Developers are not managers, and if needed, they could find workarounds, custom forks etc. Maybe saga should not accept dependencies and be as easy as possible, but sometimes it doesn't. In such cases we may delegate dependencies usage to some other object, or just make saga more complex.
So why just not have an option to use injections in sagas?

@aleksandrov
Copy link
Author

This pull request was made from incorrect branch. Will create correct one shortly

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

Successfully merging this pull request may close these issues.

5 participants