-
Notifications
You must be signed in to change notification settings - Fork 17
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
Released Episode 1, discussion: Architecture of modern Android apps with Hannes Dorfmann #1
Comments
thanks for the great podcast! |
Here you go: As already said, it's build with RxJava instead of android data-binding engine, but it should be possible to understand the "big picture" without having a good rx java knowledge. Btw. I have sent a pull request with the missing links to the show note |
Jeez, I forgot to add a bunch of links including links to Hannes profiles and projects (it was 7 am..), will do as soon as possible! @yshrsmz thank you for the feedback :) |
Good job, don`t stop. |
@artem-zinnatullin how do you keep background tasks execution during orientation changes without saving presenter? |
@AdamCopperfield usually apps I write has almost full offline support so they have kind of persistant queue for operations that need to be done, even if they were started offline, + local db or files for storing content. So in case of orientation change I just need to save But sometimes when you need to do request without using such queue I'd use @atetc thanks :) |
Great talk about modern topic, don't stop. |
Great podcast! |
So after seeing that @artem-zinnatullin is using locals db or some persistence anyway in his apps, I can understand why things might work for him.
|
First of all, thanks again for your mention and the kind words :)
Now on to the most important aspect you mentioned:
I can prove you wrong here :D |
@fabioCollini That's a nice library. I just have some nitpicking feedback: What I've meant with spaghetti code is the lack of separation of concerns. I think you can see a good example here: public class NoteViewModel extends ViewModel<String, NoteModel> {
private final Executor backgroundExecutor;
private final Executor uiExecutor;
private NoteLoader noteLoader;
private NoteSaver noteSaver;
private MessageManager messageManager;
public final ObservableBoolean loading = new ObservableBoolean();
public final ObservableBoolean sending = new ObservableBoolean();
public NoteViewModel(Executor backgroundExecutor, Executor uiExecutor, NoteLoader noteLoader, NoteSaver noteSaver, MessageManager messageManager) {
this.backgroundExecutor = backgroundExecutor;
this.uiExecutor = uiExecutor;
this.noteLoader = noteLoader;
this.noteSaver = noteSaver;
this.messageManager = messageManager;
}
@NonNull @Override public NoteModel createModel() {
return new NoteModel();
}
@Override public void resume() {
if (!loading.get() && !model.isLoaded() && getArgument() != null) {
reloadData();
}
}
public void reloadData() {
loading.set(true);
backgroundExecutor.execute(new Runnable() {
@Override public void run() {
executeServerCall();
}
});
}
private void executeServerCall() {
try {
final Note note = noteLoader.load(getArgument());
uiExecutor.execute(new Runnable() {
@Override public void run() {
model.update(note);
loading.set(false);
}
});
} catch (Exception e) {
uiExecutor.execute(new Runnable() {
@Override public void run() {
model.getError().set(true);
loading.set(false);
}
});
}
}
public void save() {
boolean titleValid = checkMandatory(model.getTitle(), model.getTitleError());
boolean textValid = checkMandatory(model.getText(), model.getTextError());
if (titleValid && textValid) {
sending.set(true);
backgroundExecutor.execute(new Runnable() {
@Override public void run() {
try {
Note note = new Note(null, model.getTitle().get(), model.getText().get());
String noteId = model.getNoteId();
if (noteId == null) {
noteId = noteSaver.createNewNote(note).getObjectId();
model.setNoteId(noteId);
} else {
noteSaver.save(noteId, note);
}
hideSendProgressAndShoMessage(R.string.note_saved);
} catch (RetrofitError e) {
hideSendProgressAndShoMessage(R.string.error_saving_note);
}
}
});
}
}
private void hideSendProgressAndShoMessage(final int message) {
uiExecutor.execute(new Runnable() {
@Override public void run() {
messageManager.showMessage(activityHolder, message);
sending.set(false);
}
});
}
private boolean checkMandatory(ObservableString bindableString, ObservableInt error) {
boolean empty = bindableString.isEmpty();
error.set(empty ? R.string.mandatory_field : 0);
return !empty;
}
@Override public ActivityResult onBackPressed() {
return new ActivityResult(true, new Note(model.getNoteId(), model.getTitle().get(), model.getText().get()));
}
} This @Gi-lo Regarding scaling, you are right MVVM can scale and you have proved that. I should have said that I, personally, am not able to scale it when working with XML bindings. It didn't worked good for me on Windows Phone and also with android data binding engine it's a little bit ugly. IMHO, it works for quickly building bring some two way data binding small UI things together like @panzerdev Yes, usually I prefer having multiple activities. As you have already said, Intents are fundamental in Android especially for deep linking. Actually, that is one things that I wish fragments would support somehow out of the box. However, we can build also a decoupled component that is responsible for navigation and deep link resolving. The nice thing of one activity is that you can animate your views much smoother as the user navigates through your app from "screen" to "screen", i.e. |
Thank you for feedback! // Interesting discussions, btw, same story as @sockeqwe, not sure I'll be able to scale MVVM project on Android. |
@sockeqwe you are right, that class is a mess :( But one reason is that the demo project doesn't use RxJava to keep it simple. Using RxJava the same class can be written in a better way: public class NoteViewModel extends RxViewModel<String, NoteModel> {
private NoteLoader noteLoader;
private NoteSaver noteSaver;
private MessageManager messageManager;
public final ObservableBoolean loading = new ObservableBoolean();
public final ObservableBoolean sending = new ObservableBoolean();
public NoteViewModel(SchedulerManager schedulerManager, NoteLoader noteLoader, NoteSaver noteSaver, MessageManager messageManager) {
super(schedulerManager);
this.noteLoader = noteLoader;
this.noteSaver = noteSaver;
this.messageManager = messageManager;
}
@NonNull @Override public NoteModel createModel() {
return new NoteModel();
}
@Override public void resume() {
if (!loading.get() && !model.isLoaded() && getArgument() != null) {
reloadData();
}
}
public void reloadData() {
subscribe(
loading::set,
noteLoader.load(getArgument()),
model::update,
t -> model.getError().set(true)
);
}
public void save() {
boolean titleValid = ValidationUtils.checkMandatory(model.getTitle(), model.getTitleError());
boolean textValid = ValidationUtils.checkMandatory(model.getText(), model.getTextError());
if (titleValid && textValid) {
subscribe(
sending::set,
noteSaver.save(model.getNoteId(), model.getTitle().get(), model.getText().get()),
note -> messageManager.showMessage(activityHolder, R.string.note_saved),
t -> messageManager.showMessage(activityHolder, R.string.error_saving_note)
);
}
}
@Override public ActivityResult onBackPressed() {
return new ActivityResult(true, new Note(model.getNoteId(), model.getTitle().get(), model.getText().get()));
}
} Maybe it continues to have too many responsibilities but it's not difficult to refactor it to delegate to other objects (for example something like a Use Case in clean architecture). How do you solve this problem in MVP? |
Please, turn off skipping silence in next records. Subsecond gaps makes it difficult to understand. |
Yeah, I understand, actually episode was about 1h 20m and we discussed some But, at the same time episode was very informative, I hope, haha :) On Tue, 9 Feb 2016, 23:16 Dmitry [email protected] wrote:
@artem_zin |
@artem-zinnatullin For the first Podcast it was quite good and understandable. Another thing: |
@Gi-lo definitely will create an issue for suggestions for the next episode soon! |
thanks for the updates!(sorry to be late I have some questions.
|
Yes, take gmail for example:
Hope that answers your question. |
btw. as some people asked me about |
@sockeqwe with gmail, if MailView button causing change a fragment, which should be done from activity or something. How will you do this operation? Activity scoped Navigator? |
Damn, I had navigation on list of topics to talk about during the podcast, but I forgot about that and we ran out of time anyway @artem-zinnatullin I don't think that Navigation is the responsibility of the Presenter. I think Navigation should be part of the "View" layer. That doesn't mean that we hardcode and couple everything in the View i.e. Fragment. What I typically have a So let's say there is the I have some kind of Navigation component (let's call this i.e. class TabletNavigator implements Navigator {
private Activity activity;
public TabletNavigator(Activity activity){
this.activity = activity;
}
@Override
public void showMail(int mailId){
MailFragment fragment = ...;
activity.getSupportFragmentManager()
.beginTransaction()
.replace(R.id.detailsContainer, fragment)
.commit();
}
...
} and then I could also have a class TabletNavigator implements Navigator {
private Activity activity;
public TabletNavigator(Activity activity){
this.activity = activity;
}
@Override
public void showMail(int mailId){
Intent i = new Intent(activity, MailDetailsActivity.class);
activity.startActivity(i);
}
...
} And then in the class InboxFragment extends Fragment {
@Inject Navigator navigator;
void onMailClicked(Mail mail){
navigator.showMail(mail.getId());
} I also think that this concept of Navigator plays quite good with shared Element transitions. We could have a method like Probably @artem-zinnatullin is doing that entirely different or have found a better way.
Not sure what exactly do you mean with scoped presenters ? Scoping depends on the complexity of your app. i.e. if I have one single |
Thank you for clarification navigation. |
First, please note that Mosby is just some kind of scaffold and you can plug in custom behaviour very easily by providing a custom delegate (see http://hannesdorfmann.com/mosby/viewstate/ scroll down until "Delegation chapter"), so you could add your custom "singleton presenter delegate" very easily. So Mosby's "default behaviour" is to have presenters that live as long as the view lives (and I don't see any reason why the presenter should live longer than the corresponding view, presenter will be garbage collected when the view gets garbage collected). However, this "default behaviour" is smart enough to determine if the view will be destroyed permanently** (i.e. user has pressed back button) or has been destroyed temporarly because of a screen orientation changes and will be recreated afterwards. With the later one, the presenter can survive screen orientation changes and the "landscape view" gets simply attached / detached from the same presenter instance as "portrait view". I believe that this "default behaviour" makes sense for 95% percent of an apps use case. So what about "singletons presenters". I don't see any advantage of implementing a presenter as "singleton" for this reasons. Have I missed something? I would like to hear what the advantages of "singleton" presenters are compared to "Mosby's" default behaviour. |
Great start, guys! How do you organize package tree with mvp? |
@b1uebyte usually I group my classes by feature like here Basically I group every screen (or subscreen like a fragment or a custom view) in is own package containing all the related classes except model classes which live in is own package like this (MVP example):
Typically I have a java gradle module for the "model" layer. So I move the "model package" from above into is own java gradle module. |
Router is still kind of "idk" thing, I tried several approaches and hadn't found clear solution yet, last one was just a class that has methods like
Here I'm opposite to @sockeqwe and prefer packaging by components
|
This is a very cool Podcast keep them coming found you guys on Android Dev Digest @sockeqwe can we apply similar pattern like Navigator for NavigationDrawer i.e. when user selects different item from the drawer . |
@sockeqwe also do you have any good example of Repository Pattern |
So I started to answer here on github your question about the Repository Pattern. It turned out that my answer is getting too long, so I decided to move my answer in a dedicated blog post: A good example (also mentioned in the blog post) can be found here: https://github.com/android10/Android-CleanArchitecture |
@sockeqwe do you use navigator pattern like thing for Navigation Drawer |
I don't think so because I don't even know what the Navigator Pattern is 😄 I mean, I do have classes called I.e. I have a interface interface Navigator {
void showNewsList();
void showNewsDetails(int newsId);
} The I would have a PhoneNavigator: class PhoneNavigator implements Navigator {
private Activity activity;
@Override
void showNewsDetails(int newsId){
Intent intent = new Intent( activity, NewsDetailsActivity.class);
intent.putExtra("NewsId", newsId);
activity.startActivity(intent);
}
...
} NewsDetailsActivity basically hosts a Then I also have a class TabletNavigator implements Navigator {
private Activity activity;
@Override
void showNewsDetails(int newsId){
activity.getFragmentManager()
.beginTransaction()
.replace(R.id.container, NewsDetailsFragment.newInstance(newsId))
.commit();
}
...
} The difference of both navigators is how they display "navigate" in the UI. PhoneNavigator starts a new Activity to display a list of news while TabletNavigator switches a Fragment in a Master / Detail view in the same Activity. And then in my activities / fragments I instantiate either a PhoneNavigator or TabletNavigator (or inject one via dagger) depending on if the device is a Phone or a Tablet. By clicking on a view somewhere in the UI I will then call Is that what you call the "Navigator Pattern"? |
Thanks that the same thing I was talking about @sockeqwe . I have one more question for you in the podcast you said that we can have more than one Presenter in our Activity for example in this image |
Use View.onAttachedToWindow() |
@sockeqwe I've a question concerning your comment of using I'm currently implementing it in a hobby project and I am using a My questions is if I reduced the state of my previous Do you have a good solution for this? I can currently only think of dirty onces. Awesome podcast btw 👍 |
The "clean" way would be to let the information that you have applied Other workarounds that may or may not work in your use case:
Go the clean way or use work around? It depends how complex your app / state is and if you are a "purist" or not :) hope that helps! |
@sockeqwe thanks for the quick response. I took a look at I thought of "clearing" the
Thanks for the response! It was more of a general wonderment I had when listening to the podcast. I'm currently doing it similarly to this https://github.com/pakoito/FunctionalAndroidReference/blob/80dd1a6d26647d3023ae234f0b7b76422eecd9b5/app/src/main/java/com/pacoworks/dereference/widgets/BaseRecyclerAdapter.java but I want to move it away from the adapter / ui thread. |
Just a reminder: Retaining the Adapter on orientation changes is a memory
leak since ViewHolders and their views has been instantiated with the
layout inflater from the very first activity. So that activity cant be
garbage collected.
Simon Vergauwen <[email protected]> schrieb am Di., 28. März 2017,
19:13:
… @sockeqwe <https://github.com/sockeqwe> thanks for the quick response. I
took a look at Mosby MVI and Mosby Conductor MVI but I have yet to give
it a try.
I thought of "clearing" the List<AdapterCommands> when detach the view
from the presenter, in a similar way as you described just at a different
point in time. My presenters are retained on config change, and are
singletons for each view. So no conflict can occur in cached view states in
the behavior relay.
dataset != null that strategy won't work for me since my adapter is also
retained on config change, I'm using Conductor and it scoped to the life of
the Controller with Dagger. Either way I'm skeptical about last 2
workarounds since.
Thanks for the response! It was more of a general wonderment I had when
listening to the podcast. I'm currently doing it similarly to this
https://github.com/pakoito/FunctionalAndroidReference/blob/80dd1a6d26647d3023ae234f0b7b76422eecd9b5/app/src/main/java/com/pacoworks/dereference/widgets/BaseRecyclerAdapter.java
but I want to move it away from the adapter / ui thread.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAjnrrqyxfaiNGlhtMq6biFPnaRpiIGtks5rqT-qgaJpZM4HUABV>
.
|
@sockeqwe thanks for the heads up. I was unaware that an Adapter kept actual references to the viewholders. I thought it was more like a sort of factory/populator. |
Hey @artem-zinnatullin @sockeqwe that is awesome podcast episode. and It clear so many things about MVP. It's really helpful if you add some good android MVP demo link ? |
It's 2.5 years old episode tho 😸 Afaik all hosts of the podcast moved past MVP since then, we want to record an episode about MVI / Reactive Redux sometime in future, but no ETA |
Yay! Feedback is really appreciated 😸
The text was updated successfully, but these errors were encountered: