yuhsinshih / SimpleTwitterClient

1 stars 0 forks source link

Review my Twitter app #1

Open yuhsinshih opened 10 years ago

yuhsinshih commented 10 years ago

Just finished the basic requirement. /cc @nesquena @thecodepath

nesquena commented 10 years ago

:+1: Nice work overall. I have provided a detailed Project 3 Feedback Guide here which covers the most common issues with this submitted project. Read through the feedback guide point-by-point to determine how you could improve your submission.

Let us know if you have any other thoughts or questions about this assignment. Hopefully by now you feel pretty comfortable with all the major pieces to basic Android apps (Views, Controllers, Models, Authentication, API Communication, Preferences, ActionBar, et al) and see how they all fit together.

yuhsinshih commented 10 years ago

Hi /cc @nesquena @thecodepath I've finished most of features for Project 4 but I'm still thinking how to achieve infinite scroll for other users' Profile view. snapshot2.gif is the current progress. Currently, the handling of infinite scroll is defined in an abstract class TweetsListFragment. I'm wondering how to get screen_name in TweetsListFragment.

nesquena commented 10 years ago

@yuhsinshih why do you need screen_name in TweetsListFragment when the populate method is defined in the UserTimelineFragment?

yuhsinshih commented 10 years ago

@nesquena currently the "infinite scroll" feature is implemented in TweetsListFragment because I thought it looks like common feature that can be shared among Fragments. I want to pass screen_name to a new onLoadMore(int page, int totalItemsCount, String screen_name) method. Is it doable or do I think wrong? What's the right direction to implement "infinite scroll" in this homework?

nesquena commented 10 years ago

@yuhsinshih scroll listener just calls customLoadMoreDataFromApi:

https://github.com/yuhsinshih/SimpleTwitterClient/blob/master/src/yhshih/apps/basictwitter/fragments/TweetsListFragment.java#L57

which just calls:

https://github.com/yuhsinshih/SimpleTwitterClient/blob/master/src/yhshih/apps/basictwitter/fragments/TweetsListFragment.java#L83

which then calls this method:

https://github.com/yuhsinshih/SimpleTwitterClient/blob/master/src/yhshih/apps/basictwitter/fragments/UserTimelineFragment.java#L31

Why not override customLoadMoreDataFromApi inside the UserTimelineFragment and call the correct populate method from there which could include the screen name?

// UserTimelineFragment
public void customLoadMoreDataFromApi(int offset) {
                if(screen_name == null)
            populateTimeline(maxId, false);
        else
            populateTimeline(maxId, false, screen_name);
}

would that work? If you can't figure it out, maybe we can look at it next time we have class (next Tuesday) since you were able to get the rest of the homework finished.

nesquena commented 10 years ago

:+1: Project looks good. I have provided a detailed Project 4 Feedback Guide here which covers the most common issues with this submitted project. Read through the feedback guide point-by-point to determine how you might be able to improve your submission.

Let us know if you have any other thoughts or questions about this assignment. Hopefully you can see this coming together as a "fully fledged" twitter client with some more work and polish. This app contains all of the components now (fragments, models, networking, client, tab navigation, image loading, et al) of 90% of dynamic data-driven API client. Obviously there are lots of details and patterns to learn, but by this point you have been introduced to all the major frameworks and concepts. Hopefully you would feel fairly confident getting started making Android apps for instagram, pinterest, yardsale, flickr, using the same patterns.