0
\$\begingroup\$

I'm trying to get and save in an ArrayList Reddit posts and comments. The code is very similar for both:

The RedditThing is a parent of RedditPost and RedditComment:

class RedditThing  {
    String author; String subreddit; String body; long score;

    public void setAuthor(String authorUsername)  {
        this.author = author; }

    public void setSubreddit(String subreddit)  {
        this.subreddit = subreddit; }

     public void setBody(String body)  {
        this.body = body; }

    public void setScore(long score)  {
        this.score = score; }

    public String getBody()  {
        return body; }

    public String getSubreddit()  {
        return subreddit; }

     public String getAuthor()  {
        return author; }

    public long getScore()  {
        return score; }

    public String toString()  {
        return subreddit + "=> " + score + "=>" + author + ": " + body; }
}

private class RedditComment extends RedditThing  {

}

private class RedditPost extends RedditThing  {
    String title;

    public void setTitle(String title)  {
        this.title = title; 
    }

    public String getTitle()  {
        return title; 
    }
}

Right now I have the getPosts method:

public ArrayList<RedditPost> getPosts(String subreddit, String postsType)  {

    ArrayList<RedditPost> result = new ArrayList<RedditPost>();
    StringBuilder link = new StringBuilder("https://oauth.reddit.com");
    link.append("/r/" + subreddit + "/" + postsType);

    try {
        URL url = new URL(link.toString());  //the same for comment 

        HttpURLConnection connection = (HttpURLConnection) url.openConnection();  //the same for comment 

        setupGETConnection(connection);  //the same for comment 
        connection.connect();  //the same for comment

        System.out.println("Done get posts");

        InputStream input = connection.getInputStream();  //the same for comment
        String inputString = new Scanner(input, "UTF-8").useDelimiter("\Z").next();  //the same for comment 

        JSONObject jsonObject = new JSONObject(inputString);  //the same for comment
        JSONArray posts = (JSONArray) ((JSONObject) jsonObject.get("data")).get("children"); 

        for (int i=0; i<posts.length(); i++)  {  //the same for comment 

            JSONObject postObject = (JSONObject) posts.get(i);  //the same for comment 

            JSONObject postData = (JSONObject) postObject.get("data");  //the same for comment
            RedditPost post  = new RedditPost();

            post.setTitle((String) postData.get("title")); 
            post.setAuthor((String) postData.get("author"));  //the same for comment 
            post.setBody((String) postData.get("selftext")); 
            post.setScore(((Integer) postData.get("score")).longValue());  //the same for comment 
            post.setSubreddit((String) postData.get("subreddit"));  //the same for comment

            result.add(post);  //the same for comment
        }

        System.out.println(inputString);  //the same for comment

       }

    catch (Exception e)  {
        System.out.println(e);  //the same for comment 
    }

    return result;  //the same for comment
}

I have commented the lines that are going to be duplicate if I create a separate getComments method.

public ArrayList<RedditComment> getComments(String thing_id)  { //different
    ArrayList<RedditComment> result = new ArrayList<RedditComment>(); //different
    StringBuilder link = new StringBuilder("https://oauth.reddit.com/r/childfree/comments/"); //different
    link.append(thing_id);

    try {
        URL url = new URL(link.toString()); 
        HttpURLConnection connection = (HttpURLConnection) url.openConnection(); 
        setupGETConnection(connection); 
        connection.connect();

        System.out.println("Done get comments");  //different

        InputStream input = connection.getInputStream();
        String inputString = new Scanner(input, "UTF-8").useDelimiter("\Z").next(); 

        JSONArray jsonArray = new JSONArray(inputString);
        JSONArray comments = (JSONArray) ((JSONObject) ((JSONObject) jsonArray.get(1)).get("data")).get("children");  //different

        for (int i=0; i<comments.length(); i++)  { 

            JSONObject commentObject = (JSONObject) comments.get(i); 
            JSONObject commentData = (JSONObject) commentObject.get("data");
            RedditComment comment  = new RedditComment();

            comment.setAuthor((String) commentData.get("author"));
            comment.setBody((String) commentData.get("body"));  //different 
            comment.setScore(((Integer) commentData.get("score")).longValue()); 
            comment.setSubreddit((String) commentData.get("subreddit"));

            result.add(comment);
        }

        System.out.println(inputString);

       }

    catch (Exception e)  {
        System.out.println(e); 
    }

    return result;
}

I was wondering if there's a better way than just creating two separate methods with a significant portion of the duplicated code.

\$\endgroup\$
1
\$\begingroup\$

Well, the methods are not that similar... basically, the real duplication is just reading a URL and parsing it into a json array, where the only difference is the message you print to stdout. As System.out.println() normally is just a temporaty solution (like: c'mon, who will read your stdout in reality?) I wager you can simply leave this out or replace it with a generic message.

Thus, create a method and use it, something along the lines of this:

JSONArray readUrlContents(String link) throws ... {
    URL url = new URL(link.toString());
    HttpURLConnection connection = (HttpURLConnection) url.openConnection();
    setupGETConnection(connection);
    connection.connect();

    System.out.println("Done reading url " + link);

    InputStream input = connection.getInputStream();
    String inputString = new Scanner(input, "UTF-8").useDelimiter("\Z").next();
    JSONObject jsonObject = new JSONObject(inputString); 
    JSONArray resultData = (JSONArray) ((JSONObject) jsonObject.get("data")).get("children"); 
    return resultData;
}

I would not go so far as to abstract the loop which comes after the reading step. You could probably do something with passing in lambdas or method references to convert the concrete object type to the respective target structure, but that would only make it complicated and convoluted just for removing the duplication of the for-loop.

While we are at it: you should close your resources after using them. (The method above does not include this part, following your example code.)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.