Skip to content
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

Add items from DD API feed #1

Merged
merged 1 commit into from
May 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,6 @@ android {
dependencies {
compile fileTree(dir: 'libs', include: ['*.jar'])
compile 'com.android.support:appcompat-v7:23.0.1'
compile 'com.squareup.okhttp3:okhttp:3.2.0'
compile 'com.google.code.gson:gson:2.6.2'
}
3 changes: 3 additions & 0 deletions app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="org.novinger.jason.demoreader" >

<uses-permission android:name="android.permission.INTERNET" />
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />

<application
android:allowBackup="true"
android:icon="@mipmap/ic_launcher"
Expand Down
73 changes: 57 additions & 16 deletions app/src/main/java/org/novinger/jason/demoreader/ListActivity.java
Original file line number Diff line number Diff line change
@@ -1,47 +1,70 @@
package org.novinger.jason.demoreader;

import android.os.StrictMode;
import android.support.v7.app.AppCompatActivity;
import android.os.Bundle;
import android.util.Log;
import android.view.Menu;
import android.view.MenuItem;
import android.view.View;
import android.view.ViewPropertyAnimator;
import android.widget.AdapterView;
import android.widget.ArrayAdapter;
import android.widget.ListView;

import com.google.gson.Gson;

import org.novinger.jason.demoreader.datamodels.Article;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;

import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;

public class ListActivity extends AppCompatActivity {

private final static int qty = 50;
private final static String feedUrl = "http://www.dailydot.com/api/v1/content/article/";

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_list);

StrictMode.ThreadPolicy policy = new StrictMode.ThreadPolicy.Builder().permitAll().build();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ORLY? I had to Google for this when I was getting exceptions about the network not working. Is there some other way I could have accomplished that?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally you check network state explicitly... http://developer.android.com/training/monitoring-device-state/connectivity-monitoring.html

the StrictMode stuff will just alert you so i don't think is useful for production

StrictMode.setThreadPolicy(policy);
Article[] articles;
try {
articles = getFeedItems(getFeedUrl());
} catch (IOException ex) {
articles = new Article[0];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this is technically fine, but FWIW primitive arrays are pretty uncommon nowadays, since they're kind of hard to work with - most people use ArrayList (which has a bunch of convenience methods), or LinkedList (if you don't need random access).

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I absolutely agree, this is more of a quick "get it to display things" snippet that I expected to come back to at some point. FWIW, it comes from the fact that I'm deserializing an array of JSON objects. I'll see if gson handles deserializing straight into an ArrayList.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly it does work when specifying a field on a POJO object, however, it does not work when calling [fromJson](http://google-gson.googlecode.com/svn/tags/1.1.1/docs/javadocs/com/google/gson/Gson.html#fromJson%28java.lang.String, java.lang.Class%29) on a gson object. :\

I've refactored this into a standalone method as much as possible in a later PR, so punting for now.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what you mean here, but will wait and see for now...

Log.d("Exception", ex.toString());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ex.getMessage()

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that shouldn't even be there anymore.

}

final ListView listview = (ListView) findViewById(R.id.listview);
String[] values = new String[] {
"Jason", "Cindy", "Hannah", "Leah", "Kirsten", "Matt", "Elle", "Trevor", "Alex",
"Eli", "Danika", "Lori", "Wyatt", "Chad"
};
final ArrayList<String> list = new ArrayList<String>();
Collections.addAll(list, values);
final ArrayList<String> list = new ArrayList<>();
for (Article article : articles) {
list.add(article.toString());
}

final ArrayAdapter adapter = new ArrayAdapter(this, android.R.layout.simple_list_item_1, list);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so for a ListView you're probably going to want to subclass BaseAdapter. that said, you might not want to spend time learning ListView and jump straight to RecyclerView (which is what we use now), which has it's own adapter class, RecyclerView.Adapter

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I just got excited about seeing things on the screen. RecyclerView is coming. :D

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\o/

listview.setAdapter(adapter);
listview.setOnItemClickListener(new AdapterView.OnItemClickListener() {
@Override
public void onItemClick(AdapterView<?> parent, final View view, int position, long id) {
final String item = (String) parent.getItemAtPosition(position);
view.animate().setDuration(500).alpha(0)
.withEndAction(new Runnable() {
@Override
public void run() {
list.remove(item);
adapter.notifyDataSetChanged();
view.setAlpha(1);
}
});
final ViewPropertyAnimator viewPropertyAnimator = view.animate().setDuration(100).alpha(0);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you understand what final is doing here?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's making the viewPropertyAnimator reference not re-assignable, correct? I was under the impression that the object could still exist and mutate, but that I could not change the reference.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's technically correct, but that's not why we're using it here. take it out and see what happens. or should i just tell you?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, let me play with it.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, I wasn't able to discern any difference in behavior.

Please correct me if I'm wrong, but since it's just a local variable (on the method), there's no other interesting behavior, right?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incorrect! you didn't see any different behavior when interacting with your ListView?

viewPropertyAnimator.withEndAction(new Runnable() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fine but is super high-level, i think API 19 or 21. i'd say the most reasonable you could go back would be 16, anything after that will exclude too many devices. a nice feature in android docs is you can set the API level, and it'll gray out anything for later APIs

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree and I didn't like it at the time. Partly because I'm not a fan of the chain-able style anymore, but partly because I didn't completely understand what I was doing or why I needed it. Unsurprisingly, that bit came from a tutorial I was looking at as I did it.

This is removed in a subsequent PR.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw you can just set a listener to do the same thing...

viewPropertyAnimator.setListener(new AnimatorListenerAdapter(){
  @Override
   public void onAnimationEnd(Blah blah...){}
});

something like that ^ which is not exactly right but shows the basic idea...

this also brings up a kind of common pattern. so the setListener expects an implementation of AnimatorListener which is just an interface. if you made an anonymous implementation there (like we do a lot), you'd have to implement 4 methods (start, end, update, whatever)... so what libraries will do a lot is have the raw interface, but also a simple implementation that just has methods that don't do anything, so you can use that and just override the methods you care about...

post back if that doesn't make sense

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah gotcha, that makes sense.

@Override
public void run() {
list.remove(item);
list.trimToSize();
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Derp, didn't mean to commit this. Was experimenting, trying to understand why used memory was increasing when a click was handled and an item removed.

I'm guessing it has something to do with GC not needing to run and new, anonymous Runnable instances being created, just to be used and thrown away. I suspect that if the list was long enough and available memory reached a certain level, a GC would occur and reclaim this memory.

Copy link
Owner Author

@jnovinger jnovinger May 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I upped the number of articles to load by default to force a larger allocation and then interacted with the activity to force more usage. As soon as it ran out of memory, it GC'd and was happy again.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can force a GC with System.gc() - it's not 100% reliable and shouldn't be used to try to free up memory (which is pointless) but is very handy for seeing what your app is doing

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good to know. I think just having access to the view of memory in use excited me. This is the same behavior I'd expect in Python, I just don't usually work with the tooling to watch memory usage in real time.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that's exactly when i use System.gc - lets say I'm doing some crazy bitmap manipulations, but i think I'm managing memory correctly - say we're on a modern tablet with a large heap - we decode the bitmap (memory), scale it (memory, memory, memory), then we made a BitmapRegionDecode (because we only want to show a part of it) (MEMORY!!!), then we return the small piece we want to show (memory), then we exit the stack (end of function, as we return the small bitmap) - we look memory monitor it probably shows 118 being used out of the 120MB we have available (because of all that bitmap allocation), but since that memory is available for GC (since we left that method's stack), if we now run a System.gc (to emulate what would happen if the system needed more memory than was available at the moment), we'd probably see that monitor dip back down to single or double digits... yay!

that all said, even if we do get to reclaim it, there's still heap limits for devices: http://stackoverflow.com/a/9940415/429430, so while we may think "oh this is temporary (on the stack), no problem", if we actually run out of memory before we're done, we'll crash - even if we catch the OOME, we'll still fail to create that small bitmap we wanted... memory management is a bitch :/

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this is awesome, thanks!

I've always wanted to work in more constrained environments. I think this will be a fun challenge at times and a good way to set any app I do apart from others in (from a quality perspective).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree

adapter.notifyDataSetChanged();
view.setAlpha(1);
}
});
}
});
}
Expand All @@ -67,4 +90,22 @@ public boolean onOptionsItemSelected(MenuItem item) {

return super.onOptionsItemSelected(item);
}

private Article[] getFeedItems(String url) throws IOException {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, ref the note above about primitive arrays - they might be appropriate here, and if it's a deliberate choice, +100, but understand the limits vs Collections (which are Lists and Sets).

Personally, I use HashSet the most, then LinkedList, then ArrayList. HashSet is super efficient, O(1) on most ops, but obv it's a Set so no duplicates

OkHttpClient client = new OkHttpClient();

Request request = new Request.Builder().url(url).build();
Response response = client.newCall(request).execute();
String json = response.body().string();
Gson gson = new Gson();
return gson.fromJson(json, Article[].class);
}

private String getFeedUrl() {
if(qty > 0) {
return feedUrl + "?quantity=" + Integer.toString(qty);
}

return feedUrl;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package org.novinger.jason.demoreader.datamodels;

/**
* Created by jason on 4/27/16.
*/
public class Article {
private String id;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the convention here is private members use the m prefix. mId. this may seem wierd, but it does allow for easy getter/setter generation...

private int mId;
public void setId(int id){
  mId = id;
}

What's nice about that too is that the private mId won't be in Javadocs, but the setter will, which will describe it's argument as id (so the user only sees the sane reading id)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, interesting. I think I misunderstood the gson docs here that I needed to name the member variables the same as the labels found in the JSON objects. Will look into this.

private int realId;
private String headline;
private String slug;
private Author[] authors;
private String summary;
private String url;

public Article() {}

public Article(String id, String headline, String slug, Author[] authors, String url, String summary) {
this.id = id;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i personally never use this.parameter = parameter - i find it a little opaque.

while this guide is for contributors, not just anyone writing android, we used it at the Dot, and we use it at O'Reilly, and i use it personally: https://source.android.com/source/code-style.html

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can also use annotations with Gson for naming styles

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is a Gson class i might actually just do what you did... i tend to let Gso POJOs be ugly :/

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was kind of my thought as well. Is the separate package, org.novinger.jason.demoreader.datamodels appropriate here to set them apart from the rest of the codebase? I wanted to signal that they were not as ... first-class as the other code.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. i do that too.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I'll get the private member renamed in a subsequent PR since I've already branched from here (twice, :\ ).

this.realId = mungeId(id);
this.headline = headline;
this.slug = slug;
this.authors = authors;
this.url = url;
this.summary = summary;
}

private int mungeId(String id) {
String[] parts = id.split(".");
return Integer.parseInt(parts[2]);
}

public String toString() {
return getHeadline() + " — " + getPrimaryAuthor().getName();
}

public int getId() {
return realId;
}

public String getHeadline() {
return headline;
}

public String getSlug() {
return slug;
}

public Author[] getAuthors() {
return authors;
}

public Author getPrimaryAuthor() {
if (authors.length > 0) {
return authors[0];
}

return new Author();
}

public String getSummary() {
return summary;
}

public String getUrl() {
return url;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package org.novinger.jason.demoreader.datamodels;

/**
* Created by jason on 4/27/16.
*/
public class ArticleFeed {
private int count;
private String next;
private String previous;
private Article[] results;

public ArticleFeed() {}

public ArticleFeed(int count, String next, String previous, Article[] results) {
this.count = count;
this.next = next;
this.previous = previous;
this.results = results;
}

public Article[] getResults() {
return results;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.novinger.jason.demoreader.datamodels;

/**
* Created by jason on 4/27/16.
*/
public class Author {
private String name;
private String url;
private String slug;

public Author() {}

public Author(String name, String url, String slug) {
this.name = name;
this.url = url;
this.slug = slug;
}

public String toString() {
return name;
}

public String getName() {
return name;
}

public String getUrl() {
return url;
}

public String getSlug() {
return slug;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package org.novinger.jason.demoreader.datamodels;

/**
* Created by jason on 4/27/16.
*/
public class Headline {
private Boolean headline;
private Boolean title;
private Boolean social;
private String text;

public Headline() {}
}