Open risayew opened 9 years ago
+1 on the question. I would like to add that the current implementation creates a new "UserListSubscriber" for example for each call to the UseCaseLayer. How would you like to handle common data (let's say that we show the UserList in another screen/widget as well)? A UserListFetchedEvent could be broadcasted to more than "one" subscriber.
A possibility is to implement an Event Bus or Event Aggregator in the Data Layer. That Bus can be injected via Dagger in a Presenter. That Presenter would then subscribe (and also unsubscribe on destruct) to certain types of Events, which are pushed up from the Data Layer.
Be aware that Event Busses come with their own downsides, as this allows loose coupling between layers and in larger apps can quickly result in event spaghetti. Also, events indicate that something has happened, not that something should happen. So a UserUpdatedEvent
means that the Presenter knows a User is updated and make a call to the Repository ( through a UseCase ) to get the new user.
@Trikke, thats where I was going.. so why not make it a standard? Use UseCases for invoking/triggering actions and subscribe to results/data via bus events?
@Trikke I will absolutely agree with you dude for the spaghetti. Here we have a big event spaghetti that's why in the new version we removed the event bus.
@erikash i wouldn't make it a standard because of my reasons against it. This repository is implemented in such a way now that allows tight coupling between components in layers and the flow of data is also tight and easy to follow from Presentation down to Data and up again.
Event busses are used for a specific set of problems and people are quick to use them to solve any of their problems since they are easy to implement.
So Event busses can be useful in the case of "Data layer lets the Presentation layer know a User was updated in a background service to we can show a toast". But they are not to be used in case of Commands, eg: "User clicks on a button to update a user". As said, events indicate that something has happened, not that something should happen.
@Trikke , I agree with you events should be in the past tense. You contradicted yourself with the example you provided... "When a user clicks on a button to update a user" should be handled via a Use Case class. The response of the updated user should be agnostic to the trigger, whether its a users "click" or a background service or another platform altogether...
Explain why the response of an action should be separated from the action itself?
The way i see it there is a difference between Events and Actions. An Event for me is just something that is thrown up to indicate a state change and notifies listeners, it is usually triggered externally. An Action is something entire different and encapsulates an entire flow of information. In the case of the button click, it is logic to have all components tightly coupled and expect a result back after an Action has happened.
So for a User clicking on a button, you would have : GUI -> domain -> data -> cloud service -> callback -> data -> domain -> GUI
If you would have a background service that polls you cloud service for updates, then Events makes sense. Background service -> cloud service -> callback -> push event -> listeners are notified
An action response most of the time would be an "event". For example, if your product has multiple platforms : web, ios, android. The used can take most of the actions on any of the platforms, those actions trigger events (state changes) which should be handled in all platforms. That's why i'm suggesting that all action "events" should be handled the same way... If you have two handlers for a user update "click" callback from: GUI -> domain -> data -> cloud service -> callback -> data -> domain -> GUI
and another for a "click" in another platform: Background service -> cloud service -> callback -> push event -> listeners are notified
Then your risking duplication bugs and other technical difficulties... Does the things i'm saying make sense?
@erikash i understand what you are trying to say, and in the end it all depends on your implementation and requirements ( and mostly, how you view it ).
But I advocate that there is a difference between Events and Actions; and that a Response is not "an event", but actually just that, a Response. Data that is returned ( be it immediately or later ) because an Action requires it to finish. A Response could hold the same information as an Event, but both are used differently.
I perfectly understand the reasons why you would architect it that way, i'm just concerned that after some time and as the project grows you'll have a hard time to figure out where and when events happened and even who triggered them. ( in my case, the external cloud service triggers it. In your case, it is buttons, external cloud service, anything actually because it is not restrictive and loose )
The architecture in this repository is implemented by Uncle Bob's clean architecture vision, so coupling is different to what you would want to achieve. I do recommend reading up on what Uncle Bob has to say.
You're right.. it all depends on the use case... all I said earlier is that the use case you pointed out is a bad example. The response callback could be "200", but I suggest that state change handler for UserUpdatedEvent should be the same regardless of the trigger. This case is actually very common for applications spanning multiple platforms.. that's why I suggest adding a solution for it in Android-CleanArchitecture.
@erikash You keep commenting that my Use Case is a bad example without ever explicitly explaining why it is so. I feel you are calling it that because it doesn't fit in your architectural plans. In my example there are certainly no duplication bugs or anything. Of course you would never just copy paste code around. Just because you have 2 separate ways to call a Cloud Service to get an updated user, this wouldn't mean that there is duplicate code anywhere. But structuring it like this, with Actions and Events, you are certain that, if something happens, you can more easily debug and find out where the problem came from, be it a button press or an event triggered from a background service. In a completely event-driven situation, this becomes much harder, since your event wouldn't be coupled to any origin.
There is no "solution" required in this repository as you suggested, as the architectural changes you suggest do not fit in the ideologies presented in this repository. I think the author ( @android10 ) can chime in on this.
You are looking to create an "event-driven" architecture, which comes with its own set of problems.
I'm completely agree with @Trikke. Actually, i meant exactly the same way of the event bus usage. And the example about external service which is scheduled in order to fetch new updates from the server is exactly my case. So, makes sense for me.
@risayew to get back to your original question. From the architectural standpoint presented in this repository, an event bus certainly can be implemented, and most probably in the Data Layer. You could have some kind of background service that would poll your Api from time to time. When an update happens, that background service would have a references to the Event Bus ( or Aggregator ) and post a UserUpdatedEvent to it. Any subscribers on the Event Bus would then be notified. Another discussion is where these subscribers should exist, but that depends on the scope of your application. Usually this will be in the Presentation Layer.
Something of note is that Events indicate that something has happen, but be careful with what data you pass with the Event. Ideally the Event holds minimal immutable data to inform subscribers. In the case of a UserUpdatedEvent, the Event holds the ID of the user that was updated, but not the User Object itself. If you would do this, the subscriber would consume the Event and use that data (eg, show the new User's name in a Toast) but might skip over a lot of Data or Business logic. The subscriber should use the ID passed by the event to get the new User through a Use Case. This way, you pass by your business rules and any Data implementation that might exist. For example, a business rule states that a user with a long name should verify if this name is correct. But if the subscriber in the Presenter consumes an entire User object and just shows the Toast, the business rules that are applied when getting a User through a Use Case are not enforced. ( keep in mind, i'm just making up silly rules here )
I hope i explained the matter clearly.
@Trikke, thank you very much for the explanation. It's great! In my project i don't pass the data from the data layer to the presentation via the event bus. Only the boolean flag indicating, that information in some table has been updated. Suppose, i have 2 tables: "news" and "infos". My Updater service updates the database periodically. And if, for example, information in the "news" table has been updated, then corresponding event: ContentUpdatedEvent with the flags set to newsUpdated=true, infosUpdated=false will be posted on the event bus. Views, which are interested in this information are subscribed to the event bus. If some view interested in news information, then GetNewsList use case will be activated. In general, i can say, that i use event bus only in order to pass events from the data layer to the presentation layer. Events, but not the data.
@risayew yup, that's the general idea of it! The Presentation Layer receives the Event and reacts to it by invoking the Domain Layer. Just make sure to keep away from writing code to make an Event happen because you need the logic that is executed as a response to that Event to happen for entirely different reasons than that logic was written. For example : you have a ContentUpdatedEvent, to which Views react and get the new data through the Domain. Now imaging you make a Button the user can press to immediately try to get new News or Infos. This button should just call trough to the Domain to get the information required. You should not write code that would trigger your Updater, to trigger the ContentUpdatedEvent to get the Views to update. This might seem logical in a way because you are making the system fire Events and reacting to them. But in this instance, you'll end up spending lots of time debugging when your Updater fails and you have no idea where or why it was triggered. (Did the user click that button, did he click it twice, did he click it and close the app, did the update in the background thread fail). Now imagine a much more complex system like this and you have Event Spaghetti.
Nice discussion here! Just my 2 cents here: As @Trikke pointed out, do not confuse Events with Actions. Of course with an EventBus we are totally decoupled, however, I think they work the same way as GOTO instruction (https://en.wikipedia.org/wiki/Goto), so careful must be taken since we can loose control of our flow and this will bring as a consequence, hard debugging, and for instance difficulties when solving bugs.
@Trikke What if the Domain Layer will receive the evant (ContentUpdatedEvent) and reacts to it by invoking the Use Case (notify Presentation Layer to show progress if needed, than do some business logic, get refreshed data) then update refreshed data in Presentation Layer. In this flow we don't skip business logic and Presentation don't know about push event. It is a good idea or not?
@justplay1 This discussion is months old, so i'd advice going through it entirely to read up on the point. Because what you seem to be suggesting feels wrong. This discussion was about the Data Layer having a background service that would push events up.
A Use Case can't notify the Presentation Layer to show progress, it has no reference to it. So if you would have an event listener in your Domain Layer, the only thing you can do is run business logic. From that layer you cannot access the Presentation Layer, so you cannot inform your user of any updates, or update data in the UI. A listener in the Domain layer would only make sense if it is important to your business logic.
For example : A background service pushes an event to tell us that the app configuration was updated. Domain listens to this, gets the updated configuration and runs the required business logic that could be involved. The user has no need (and can not) to know about this.
But even then, i think there are better ways to accomplish this then with "event-spagetti".
@Trikke , thank you for the explanation
What about update events in the data layer and how to notify about them the presentation layer? Let's suppose, that some third party service has updated some user data in the data base.
Is event bus approach good for that? For example, we will define some event class in the domain layer: UserUpdatedEvent. Some presenter in the presentation can subscribe/unsubscribe for this event on the event bus. Data layer can post the UserUpdatedEvent on event bus ...