Please review my app [Day1 + Day2 + Day3 + Day4]

[houjieth]

@codepath @codepathreview

@nickai

Sorry for my late submission. I swear I will catch it up starting Day 3.

[nickai]

Thanks Jie for getting this in! Let us know if you need any help with day 3 as it is the toughest day of the assignment. We've captured some of the best practices for the day 2 assignment below. We recommend you take a look through these point-by-point to determine how you might be able to improve your submission.

• Did you create helper methods for Activity initialization in onCreate? This keeps lifecycle methods clean and increases readability, especially as your Activity increase in complexity.

• Loading adapter content. It's preferred that a convenience method be created on the adapter to reload content. This helps DRY up code and encapsulate adapter behavior if the Activity needs to call this several times in different places.

  public class CommentsAdapter extends RecyclerView.Adapter<CommentsAdapter.ViewHolder> {
  
   List<Comment> comments;
  
   public void refreshComments(List<Comment> newComments) {
       comments = newComments;
       notifyDataSetChanged();
    }
  }

• In RecyclerView.Adapter subclasses, context can be retrieved from either a view holder or the parent view. See this guide on Accessing context from RecyclerView.Adapter

• Attach any listeners in the ViewHolder to cut down on memory usage. Rather than setting OnClickListener instances in RecyclerView.Adapter.onBindViewHolder(), which causes more objects to be created, do the following:

  public class InstagramPostsAdapter extends RecyclerView.Adapter<InstagramPostsAdapter.ViewHolder> {
    @Override
    public void onBindViewHolder(ViewHolder holder, int i) {
        InstagramPost post = posts.get(i);
        holder.post = post;
    }
  
    public static class ViewHolder extends RecyclerView.ViewHolder {
        private InstagramPost post;
        private Button btnDoSomethingWithPost;
  
        public ViewHolder(View itemView) {
            super(itemView);
            btnDoSomethingWithPost = (Button) itemView.findViewById(R.id.btnDoSomethingWithPost);
            setupListeners();
        }
  
        void setupListeners() {
            btnDoSomethingWithPost.setOnClickListener(new View.OnClickListener(){
                @Override
                public void onClick(View v) {
                    // context can be referenced via v.getContext()
                    doSomethingWithPost(post);
                }
            });
        }
    }
  }

• Fetching data from the Instagram API to populate RecyclerViews
  • Did you setup your InstagramClient with an endpoint such as getPopularPosts? You should have a method that represents each endpoint and then accepts specific parameters and a “handler” as outlined here.
  • You should then make a call to getPopularPosts in your activity and the onSuccess should deserialize the response and then use it to populate a RecyclerView.Adapter.
• Handling Network Failures
  • When making network requests, it's important to handle error cases and take appropriate actions when the network request fails. For this reason, it is advisable to override the onFailure method in JsonHttpResponseHandler, and at a minimum, notifying the user via a Toast that the network request failed.

[houjieth]

@nickai @codepath @codepathreview Sorry, just finish day 3. 8 hours of Android coding plus company work, super crazy day!

[houjieth]

Please review my work for the whole Instagram client. @nickai @codepath @codepathreview

I only implemented the compulsory user stories.

[nickai]

Thanks Jie for wrapping up the assignment over the weekend. I've included the feedback guides for day 3 and 4 below for you to check out.

• Using a global helper method to retrieve the InstagramClient instance in the Application class

    public static InstagramClient getRestClient() {
        return (InstagramClient) InstagramClient.getInstance(InstagramClient.class, sharedApplication());
    }

• Helpful Fragment methods
  • getActivity()
  • getActivity() returns the parent activity.
  • Can be called anytime after the onAttach() callback method has been run.
  • getView()
  • getView() returns the root view returned by onCreateView
  • Will return null if called within or before onCreateView()
  • Safe to call during or after onViewCreated()

• When fetching items to populate a RecyclerView.Adapter, it's better to update the the dataset rather than creating a new adapter on each fetch.

    // In initialization of the fragment or activity
    mInstagramPostsList = new ArrayList<Post>();
    mInstagramPostsAdapter = new InstagramPostsAdapter(mInstagramPostsList);
    mInstagramRecyclerView.setAdapter(mInstagramPostsAdapter);
  
    // Fetching posts in the user's feed
    instagramClient.getSelfFeed(new JsonHttpResponseHandler() {
        @Override
        public void onSuccess(int statusCode, Header[] headers, JSONObject response) {
            // Resets the list of posts and calls notifyDataSetChanged()
            mInstagramPostsAdapter.reloadPosts(Utils.decodePostsFromJsonResponse(response));
        }
    };

• Fragment newInstance method only necessary to initialize data
  • If no initialization data is expected for a fragment, then using the default no args constructor is sufficient. MyFragment.newInstance() is only used to set the fragment's argument bundle so that any initialized data is restored on fragment re-creation.
• Did you add a ProgressBar for any long running tasks, such as fetching data via the network?
  • It's always a good idea to display an indeterminate or determinate progress bar to the user when potentially long running tasks are happening in the background, especially those that update views. See this guide on implementing a ProgressBarView.

• Hardcoding integer dimensions (not sp or dp)

  • If hardcoding an integer that is used to provide spacing, it's ideally better to supply a value in dimens.xml as an integer to allow different orientations/screen sizes to supply different values.
  • And referring to it in code via getResources().getInteger(R.integer.vertical_spacer_height). E.g.,

  // in dimens.xml
  <resources>
    <dimen name="some_padding">16dp</dimen>
    <integer name="vertical_spacer_height">24</integer>
  </resources>

• Almost never call getBaseContext()
  • getBaseContext() returns the context that's wrapped inside an Activity. An Activity is a ContextWrapper subclass. it's outside the scope of this class, but usually you'd only use getBaseContext() if you want a context without activity specific styles applied.
  • If you want a context that is not tied to the activity lifecycle, then use activity.getApplicationContext() instead.
• AsyncHttpClient get method with no RequestParams
  • Theres a client.get(String url, ResponseHander handler) signature.
  • see AsyncHttpClient Javadocs
• What operations should occur in the Service
  • We definitely want to make the network request in the service.
  • While we're already in the service context, which runs on a background thread, you should do the following in the service as well:
  • parse the network response when it comes back
  • load the parsed objects into the DB
  • return the parsed objects to any listeners
• Default value for a Service's exported attribute
  • When declared in the AndroidManifest.xml file, a Service is by default not accessible by other applications (i.e., exported = false) if it has no intent filters. See Service elements in Manifest File for a detailed explanation.
• Avoid nested ViewGroups. Flatter view hierarchies are more performant
  • If possible, try to keep the view hierarchy as flat as possible. Nesting ViewGroups should be avoided, since this can affect the time that it takes to render the layout. See the following article on how Android draws views. A good number of ViewGroups do multiple traversals of their view hierarchies to calculate how much screen real estate all of its contents should receive.
• When clearing rows in the DB, foreign key constraints must be taken into account
  • Because posts reference user and image rows, you cannot delete a user if the DB contains a post that references it.
  • Emptying the database needs to be done in this order:

  // this is the join table for posts and comments
  db.delete(TABLE_POSTS_COMMENTS, null, null);

  // posts reference both users and images
  db.delete(TABLE_POSTS, null, null);

  db.delete(TABLE_COMMENTS, null, null);

  // users and images refer to no other tables
  db.delete(TABLE_IMAGES, null, null);
  db.delete(TABLE_USERS, null, null);

• Wrap transaction operations in a try / finally block
  • To ensure that db.endTransaction() is called regardless of whether an operation fails or not:

   db.beginTransaction();
   try {
     ...
     db.setTransactionSuccessful();
   } finally {
     db.endTransaction();
   }

• Convert InstagramPost class to Parcelable for easy array serialization
  • Because the InstagramPost class implements Serializable, to package a list of posts into an intent, a serializable wrapper is needed. This is fine for the assignment.
  • Parcelable: A cleaner alternative
  • The problem with InstagramPost is that it contains nested objects. See this article, which shows how nested parcelable objects are marshalled and demarshalled.
  • Once InstagramPost implements Parcelable instead of Serializable, you can do this:

  List<InstagramPost> posts = new ArrayList<InstagramPost>();

  Intent intent = new Intent();
  intent.putExtra("posts", posts);

  // Extracting data on receipt of intent
  List<InstagramPost> fetchedPosts = getIntent().getParcelableArrayListExtra("posts");