Hw3 ready for review

[upkarlidder]

I finished the basic requirements. I will not be able to do persistence today. I will put it in later. Wanted to submit for marking. Thank you both /cc @nesquena @timothy1ee

[nesquena]

:+1: Nice work Upkar, a few points of feedback:

• Properly leveraged RestClientTemplate and configured TwitterClient to authenticate and send requests to API
• Leveraged UniversalImageLoader to load all remote images
• Setup login with OAuthLoginActivity and handled onLoginSuccess
• Properly setup Tweet and User models extracting relevant data from JSON
• As a rule of thumb, you can move more of the onCreate setup into separate methods. Think of onCreate as a place to invoke methods from but probably shouldn't have too much code in it directly. Also, https://github.com/8indaas/twitterclone/blob/master/src/com/codepath/upkar/twitterapp/TimeLineActivity.java#L54 and https://github.com/8indaas/twitterclone/blob/master/src/com/codepath/upkar/twitterapp/TimeLineActivity.java#L93 is essentially identical code (except a boolean flag, could have been one method).
• One minor suggestion, in terms of parameter order I would put everything before the handler. For example here: https://github.com/8indaas/twitterclone/blob/master/src/com/codepath/upkar/twitterapp/TwitterClient.java#L53 I would put status before handler for better readability.
• Properly setup home timeline request through TwitterClient and tweets are displayed in timeline ListView
• You should consider handling the onFailure case as well as onSuccess when working with any async requests. Otherwise things will be totally unhandled and broken without internet or if a request fails.
• Setup compose window, displaying relevant information and properly posting tweet
• Setup character counter for compose window (limited to 140 characters)
• Properly display tweet in timeline after the compose succeeds by returning
• Implemented endless pagination for a smooth feed consumption experience

I noticed you played around with transitions and a few nice UI touches, nice work. As you mentioned would encourage you to play with ActiveAndroid and offline persistence as well but other than that, the project looks great. 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.

[upkarlidder]

Thanks for feedback Nathan. I will try and complete the app with offline persistence today. and also refactor methods like you suggest. For onFailure, is a Toast.show() a good option?

I have another question. The async request to twitter has multiple success handlers. And it seems the one that is called depends on the result (JSONObject vs JSONArray). I need to get the twitter user name when the timeline activity starts. I want to store in the preferences and then use it in the compose view. Right now, I have hardcoded it as my onSuccess method is never called. Please see

https://github.com/8indaas/twitterclone/blob/master/src/com/codepath/upkar/twitterapp/TimeLineActivity.java#L36 and L37 for the hardcoded value.

The method that makes the call is https://github.com/8indaas/twitterclone/blob/master/src/com/codepath/upkar/twitterapp/TimeLineActivity.java#L65

and the API call is https://github.com/8indaas/twitterclone/blob/master/src/com/codepath/upkar/twitterapp/TwitterClient.java#L59

I will be a little early to class today and can ask you then. Thank you both.

[nesquena]

Hi Upkar,

Casually inspecting the issue, I suspect it's probably https://github.com/8indaas/twitterclone/blob/master/src/com/codepath/upkar/twitterapp/TwitterClient.java#L61

Notice how https://github.com/8indaas/twitterclone/blob/master/src/com/codepath/upkar/twitterapp/TwitterClient.java#L54 we invoke 'getApiUrl' to expand the relative url string.

In the second case you haven't expanded the url string using 'getApiUrl' and that's why you are likely running into problem because it's not sending the request to the qualified path (so it's never hitting twitter)

[upkarlidder]

Thank you. I will chalk this up to "too much staring at the screen" and save myself the embarrassment :). See you tonight.

[nesquena]

For onFailure, is a Toast.show() a good option?

Yeah that's certainly not a bad approach in a case with no better fallback. Toast is the equivalent to a js alert, it gets the job done. You might imagine in a "production" app that you might add a little header to the ListView warning them their internet is failing and to try again.