Applying shaky grasp of design patterns - preparing for multithreading & spring

[Sean Funk]

I've set out on writing my first end-to-end java app that utilizes spring & mysql, in the hopes of extending this to Android development in the near future. However, I've still got a ways to go I think and there's still some concepts which are murky to me.

I've refactored the code a couple of times now, but I'm kind of confused as to how I should proceed to implement multithreading. I'm under the assumption that the preferred methodology is using a pre-determined executorpool (threadpool?) and then starting - but I'm not sure that doing this from the main method itself is the right way to do it. Here are the two classes and java file:

package com.blah.dev;

import org.joda.time.DateTime;
import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
import org.jsoup.nodes.Element;

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


/**
 * Created byon 6/29/14.
 */

class ForumPageDataHandler {

    public static void main(String[]args) throws IOException {

        List<String> postRows;
        postRows = DataHandler.getForumPage();
        List<ForumPost> forumPosts = new ArrayList<ForumPost>();
        Integer postRowCount = 0;
        System.out.println("start: " + new DateTime().toString());

        // DEBUG print how many cores are available.
        Integer cores = Runtime.getRuntime().availableProcessors();
        System.out.println("Available cores: " + cores.toString());

        // Iterate over the postRows array and parse each post into another set of arrays
        // TODO: I think this may be where a thread should start to be spawned.
        for (String postRow : postRows) {
            ForumPost forumPost = new ForumPost();
            Document postRowDoc = Jsoup.parse(postRow);
            Element postRowElement = postRowDoc.body();

            DataHandler.postRowWorker(postRowElement, postRowDoc, forumPost);
            DataHandler.postBodyWorker(forumPost.getpId().toString(), forumPost);
            forumPosts.add(forumPost);
            postRowCount++;
        }
        System.out.println("end: " + new DateTime().toString());
        System.out.println("\nTotal rows processed: " + postRowCount.toString());
        System.out.println("Total parsed forum posts: " + forumPosts.size());
    }
}

package com.blah.dev;

import org.apache.commons.io.FilenameUtils;
import org.joda.time.DateTime;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;
import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
import org.jsoup.nodes.Element;
import org.jsoup.nodes.Node;
import org.jsoup.nodes.TextNode;
import org.jsoup.select.Elements;

import javax.imageio.ImageIO;
import javax.xml.ws.http.HTTPException;
import java.awt.image.BufferedImage;
import java.io.*;
import java.net.URL;
import java.util.*;

/**
 * Created by on 7/6/14.
 */
public class DataHandler {

    public static List<String> getForumPage() throws IOException {
        String clPoWo = "https:///?act=DF&forumID=20&areaID=9";
        String document = Jsoup.connect(clPoWo).timeout(10000).get().getElementsByClass("threads").html();
        List<String> postRows = new ArrayList<String>(Arrays.asList(document.split("<br />")));
        return sanitizePostRows(postRows);
    }

    public static void postRowWorker(Element postRowElement, Document postRowDoc, ForumPost forumPost) {
        forumPost.setPostPoints(pointsHandler(postRowElement));
        HashMap postLocations = locationHandler(postRowDoc, postRowElement);
        forumPost.setpLocation(postLocations);
        forumPost.setpId(idHandler(postRowDoc));
    }

    public static void postBodyWorker(String postId, ForumPost forumPost) throws IOException {
        String clPoWoPostBody = "https:///?act=Q&ID=";
        clPoWoPostBody = clPoWoPostBody.concat(postId);
        Document doc = Jsoup.connect(clPoWoPostBody).timeout(10000).get();
        Elements bodyContent = doc.getElementsByClass("quote");

        // Set title.
        forumPost.setpTitle(doc.select("span:eq(0)").text());

        // Set handle.
        forumPost.setpHandle(doc.select("div#titleLine, b").get(2).text());

        // Settings date time. Sometimes I pick things up I should, and the string needs cleaning.
        forumPost.setpDateTime(dateTimeHandler(doc));

        // Setting body
        if (!bodyContent.select("span.fbod").text().isEmpty()) {
            forumPost.setpBody(bodyContent.select("span.fbod").text());
        } else forumPost.setpBody(null);

        // Storing links independently in a list
        if (bodyContent.select("span.fbod").select("[href]").hasText()) {
            forumPost.setpLinks(linkHandler(bodyContent));
        }

        // Grabbing and setting images
        if (imgURLHandler(bodyContent).size() > 0) {
            System.out.println("Image download start: " + new DateTime());
            try {
                forumPost.setpImages(postImageHandler(forumPost, imgURLHandler(bodyContent)));
            } catch (Exception e) {
                e.printStackTrace();
            }
            System.out.println("End download: " + new DateTime());
        }
    }

    // This removes the first (0) and the last (N) entries in the arraylist because when we define postRows
    // the code I'm using grabs more than it should.
    public static List<String> sanitizePostRows(List<String> postRows) {
        int postRowsSize = postRows.size();
        postRows.remove(postRowsSize - 1);
        postRows.remove(0);
        return postRows;
    }

    // Grab and return post id.
    public static Integer idHandler(Document postRowDoc) {
        Element postIdElement = postRowDoc.select("a").first();
        String postIdString = postIdElement.attr("href").replace("?act=Q&ID=", "");
        return Integer.parseInt(postIdString);
    }

    // Grab and return post links.
    public static List<String> linkHandler(Elements bodyContent) {
        List<String> postLinks = new ArrayList<String>();
        Elements postLinksElements = bodyContent.select("span.fbod").select("[href]");
        for (Element link : postLinksElements) {
            String links = link.select("a[href]").text();
            postLinks.add(links);
        }
        return postLinks;
    }

    // grab and return image links from post.
    public static List<String> imgURLHandler(Elements bodyContent) {
        List<String> postImageURLs = new ArrayList<String>();
        Elements postImageElements = bodyContent.select("span.fbod").select("img");
        for (Element img : postImageElements) {
            String imgUrl = img.select("img").attr("src");
            postImageURLs.add(imgUrl);
        }
        return postImageURLs;
    }

    // Parse post points and return hashmap of positive/negative points.
    public static HashMap<String, Integer> pointsHandler(Element postRowElement){
        HashMap<String, Integer> postPoints = new HashMap<String, Integer>();
        if (postRowElement.select("font > b").hasText()) {
            String pointString;
            pointString = postRowElement.select("font > b").text().replace("+", "");
            Integer pointInt = Integer.parseInt(pointString);
            postPoints.put("posPoints", pointInt);
        }
        if (postRowElement.select("em").hasText()) {
            String pointString;
            pointString = postRowElement.select("em:contains(-)").text().replace("-", "");
            Integer pointInt = Integer.parseInt(pointString);
            postPoints.put("negPoints", pointInt);
        }
        return postPoints;
    }

    // Handles parsing the datetime string and returns a properly formatted datetime to the caller.
    public static DateTime dateTimeHandler(Document doc) {
        String postDateTime = doc.select("font").text();
        if (postDateTime.length() != 11) {
            postDateTime = postDateTimeCleanup(doc, postDateTime);
            return convertPostDateTime(postDateTime);
        }
        return convertPostDateTime(postDateTime);
    }

    // Convert the postDateTime string to datetime compatible with MySQL datetime field.
    public static DateTime convertPostDateTime(String postDateTimeString) {
        if (postDateTimeString.length() > 11) {
            postDateTimeString = postDateTimeString.substring((postDateTimeString.length() - 11), postDateTimeString.length());
        }
        int currentYear = Calendar.getInstance().get(Calendar.YEAR);
        DateTimeFormatter clFormat = DateTimeFormat.forPattern("MM/dd HH:mm");
        return DateTime.parse(postDateTimeString, clFormat).withYear(currentYear);
    }

    // Method to clean up postDateTime strings that are not correct. We can fix strings that are too long, but too
    // short a postDateTime means we can't provide one, so a default should be returned.
    private static String postDateTimeCleanup(Document doc, String postDateTime) {
        if (postDateTime.length() > 11) {
            System.out.println("WARNING: postDateTime STRING IS LONGER THAN 11 CHARS: " + postDateTime);
            System.out.println("Handle causing issue: " + doc.select("div#titleLine, b").get(2).text());
            int dateSep = postDateTime.indexOf("/");
            if (dateSep > 2) {
                postDateTime = postDateTime.substring((postDateTime.length() - 11), 13);
            }
            System.out.println(postDateTime);
        } else {
            System.out.println("WARNING: postDateTime STRING IS SHORTER THAN 14 CHARS: " + postDateTime);
            System.out.println("Handle causing issue: " + doc.select("div#titleLine, b").get(2).text());
            postDateTime = new DateTime().toString(); // Returning a datetime to avoid a break.
        }
        return postDateTime;
    }

    // Image handler method. Downloads images, returns a hashmap of filename(s) and image(s).
    public static HashMap<String, BufferedImage> postImageHandler(ForumPost postData, List<String> postImgUrls) {
        HashMap<String, BufferedImage> postImages = new HashMap<String, BufferedImage>();
        String timestamp = timeStamper();
        File imgFolder = new File(timestamp);
        new File("img/" + imgFolder.toString()).mkdirs();
        for (String imgUrlString : postImgUrls) {
            try {
                URL imgUrl = new URL(imgUrlString);
                String filename = FilenameUtils.getName(imgUrl.toString());
                InputStream inputImgStream = imgUrl.openStream();
                /*String mimeType;
                // This if statement does not work. mimeType returns null. In observed cases,
                if (!FilenameUtils.isExtension(filename, postData.getImgMimeList())) {
                    mimeType = URLConnection.guessContentTypeFromStream(inputImgStream);
                    filename.concat("." + mimeType);
                }*/
                OutputStream outputImgStream = new FileOutputStream("img/" + imgFolder.toString() + "/" + filename);
                byte[] buffer = new byte[2048];
                int length = 0;
                while ((length = inputImgStream.read(buffer)) != -1) {
                    outputImgStream.write(buffer, 0, length);
                }
                BufferedImage bfrImg = ImageIO.read(inputImgStream);
                inputImgStream.close();
                outputImgStream.close();
                postImages.put(filename, bfrImg);
            } catch (IOException ioException) {
                System.out.println("There was a problem with the image handling logic.");
                ioException.printStackTrace();
            } catch (HTTPException e) {
                e.getStatusCode();
            }
        }
        return postImages;
    }

    // This method parses out location data.
    public static HashMap<String, String> locationHandler(Document postRowDoc, Element postRowElement) {
        HashMap<String, String> postLocation = new HashMap<String, String>();
        if (postRowDoc.select("span[title]").hasAttr("title")) {
            postLocation.put("locationLong", postRowDoc.select("span[title]").attr("title"));
            postLocation.put("locationShort", postRowDoc.select("span[title]").text());
        } else {
            // The following loop i used to obtain the text string for responses, which is a string as opposed
            // to an element.
            Integer tNodeIndex = 0;
            for (Node tNode : postRowElement.childNodes()) {
                if (tNode instanceof TextNode) {
                    if (tNodeIndex == 0) {
                        postLocation.put("locationResponse", tNode.toString());
                        break;
                    }
                    tNodeIndex++;
                }
            }
        }
        return postLocation;
    }

    public static String timeStamper() {
        DateTime dt = new DateTime();
        DateTimeFormatter fmt = DateTimeFormat.forPattern("yyyyMMddHHmmssSSS");
        return fmt.print(dt);
    }

}

package com.blah.dev;

import org.joda.time.DateTime;

import java.awt.image.BufferedImage;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;

/**
 * Created by on 6/24/14.
 */

public class ForumPost {

    private HashMap<String, String> pLocation = new HashMap<String, String>();
    private java.lang.String pTitle = new String();
    private DateTime pDateTime = new DateTime();
    private java.lang.String pHandle = new String();
    private Integer pId;
    private java.lang.String pBody = new String();
    private HashMap<String, BufferedImage> pImages = new HashMap<String, BufferedImage>();
    private List<String> pLinks = new ArrayList<String>();
    private HashMap<String, Integer> postPoints = new HashMap<String, Integer>();
    private List<String> imgMimeList = new ArrayList<String>();

    public ForumPost() {
    }

    public List<String> getImgMimeList() {
        return imgMimeList;
    }

    public HashMap<String, String> getpLocation() {
        return pLocation;
    }

    public void setpLocation(HashMap<String, String> pLocation) {
        this.pLocation = pLocation;
    }

    public String getpTitle() {
        return pTitle;
    }

    public void setpTitle(String pTitle) {
        this.pTitle = pTitle;
    }

    public DateTime getpDateTime() {
        return pDateTime;
    }

    public void setpDateTime(DateTime pDateTime) {
        this.pDateTime = pDateTime;
    }

    public String getpHandle() {
        return pHandle;
    }

    public void setpHandle(String pHandle) {
        this.pHandle = pHandle;
    }

    public Integer getpId() {
        return pId;
    }

    public void setpId(Integer pId) {
        this.pId = pId;
    }

    public String getpBody() {
        return pBody;
    }

    public void setpBody(String pBody) {
        this.pBody = pBody;
    }

    public HashMap<String, BufferedImage> getpImages() {
        return pImages;
    }

    public void setpImages(HashMap<String, BufferedImage> pImages) {
        this.pImages = pImages;
    }

    public List<String> getpLinks() {
        return pLinks;
    }

    public void setpLinks(List<String> pLinks) {
        this.pLinks = pLinks;
    }

    public HashMap<String, Integer> getPostPoints() {
        return postPoints;
    }

    public void setPostPoints(HashMap<String, Integer> postPoints) {
        this.postPoints = postPoints;
    }

}

Am I utilizing the concept of a class properly in this example? I feel like I don't quote have a clear separation of "duties" between the DataHandler and ForumPageDataHandler (could be renamed to match the package I guess..) classes. My goal is to effectively add multithreading at the for loop in the main[] class, since that would handle individual "rows", with no individual row being dependent on any other, with each thread just adding it's completed object to forumPosts at the end. This is where I would hand the arraylist off to a batch for insertion into a mysql db (spring integration, I believe).

Thanks for reading, and I apologize if my question isn't clear enough. I might have been staring at this for too long. Also, I just found a couple of bugs but they are minor (imgMimeList is one).

[Sean Funk]

Maybe this was in the wrong forum, but I don't want to double post, so here's an update:

Refactored some more. Removed one file completely and I think I'm much closer to being able to properly implement Runnable, but I'm still a little murky about where I create the threads and where I assign the work to the threads. Updated code:

package com.blah.dev;

import org.joda.time.DateTime;
import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
import org.jsoup.nodes.Element;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

/**
 * Created by blah on 6/29/14.
 */

class ForumPageDataHandler implements Runnable {

    public static void main(String[]args) throws IOException {

        final List<String> postRows = forumGetter();
        final List<ForumPost> forumPosts = new ArrayList<ForumPost>();
        Integer postRowCount = 0;
        System.out.println("start: " + new DateTime().toString());

        // DEBUG print how many cores are available.
        final Integer cores = Runtime.getRuntime().availableProcessors();
        System.out.println("Available cores: " + cores.toString());

        final ExecutorService executorPool = Executors.newFixedThreadPool(cores);

        // Iterate over the postRows array and parse each post into another set of arrays
        // TODO: I think this may be where a thread should start to be spawned.
        for (String postRow : postRows) {
            ForumPost forumPost = new ForumPost();
            Document postRowDoc = Jsoup.parse(postRow);
            Element postRowElement = postRowDoc.body();
            forumPost.postRowWorker(postRowElement, postRowDoc, forumPost);
            forumPost.postBodyWorker(forumPost);
            forumPosts.add(forumPost);
            postRowCount++;

            /*Runnable rowThread = new Runnable() {
                @Override
                public void run() {
                    ForumPost forumPost = new ForumPost();
                    Document postRowDoc = Jsoup.parse(postRow);
                    Element postRowElement = postRowDoc.body();
                    forumPost.postRowWorker(postRowElement, postRowDoc, forumPost);
                    forumPost.postBodyWorker(forumPost);
                    forumPosts.add(forumPost);
                    postRowCount++;
                }
            }*/
        }

        System.out.println("end: " + new DateTime().toString());
        System.out.println("\nTotal rows processed: " + postRowCount.toString());
        System.out.println("Total parsed forum posts: " + forumPosts.size());
    }

    static List<String> forumGetter() throws IOException {
        String clPoWo = "https://forums.craigslist.org/?act=DF&forumID=20&areaID=9";
        String document = Jsoup.connect(clPoWo).timeout(10000).get().getElementsByClass("threads").html();
        List<String> postRows = new ArrayList<String>(Arrays.asList(document.split("<br />")));
        postRows = sanitizePostRows(postRows);
        return postRows;
    }

    static Document postGetter(String postId) throws IOException {
        String clPoWoPostBody = "https://forums.craigslist.org/?act=Q&ID=";
        clPoWoPostBody = clPoWoPostBody.concat(postId);
        Document doc = Jsoup.connect(clPoWoPostBody).timeout(10000).get();
        return doc;
    }

    // This removes the first (0) and the last (N) entries in the arraylist because when we define postRows
    // the code I'm using grabs more than it should.
    private static List<String> sanitizePostRows(List<String> postRows) {
        int postRowsSize = postRows.size();
        postRows.remove(postRowsSize - 1);
        postRows.remove(0);
        return postRows;
    }

    /**
     * When an object implementing interface <code>Runnable</code> is used
     * to create a thread, starting the thread causes the object's
     * <code>run</code> method to be called in that separately executing
     * thread.
     * <p/>
     * The general contract of the method <code>run</code> is that it may
     * take any action whatsoever.
     *
     * @see Thread#run()
     */
    @Override
    public void run() {

    }
}

package com.blah.dev;

import org.apache.commons.io.FilenameUtils;
import org.joda.time.DateTime;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;
import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
import org.jsoup.nodes.Element;
import org.jsoup.nodes.Node;
import org.jsoup.nodes.TextNode;
import org.jsoup.select.Elements;

import javax.imageio.ImageIO;
import javax.xml.ws.http.HTTPException;
import java.awt.image.BufferedImage;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URL;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

/**
 * Created by blah on 6/24/14.
 */

public class ForumPost {

    private ConcurrentMap<String, String> pLocation = new ConcurrentHashMap<String, String>();
    private java.lang.String pTitle;
    private DateTime pDateTime = new DateTime();
    private java.lang.String pHandle;
    private Integer pId;
    private java.lang.String pBody;
    private ConcurrentMap<String, BufferedImage> pImages = new ConcurrentHashMap<String, BufferedImage>();
    private List<String> pLinks = new ArrayList<String>();
    private ConcurrentMap<String, Integer> postPoints = new ConcurrentHashMap<String, Integer>();
    private List<String> imgMimeList = new ArrayList<String>();

    public ForumPost() {
    }

    public List<String> getImgMimeList() {
        return imgMimeList;
    }

    public ConcurrentMap<String, String> getpLocation() {
        return pLocation;
    }

    private void setpLocation(ConcurrentMap<String, String> pLocation) {
        this.pLocation.putAll(pLocation);
    }

    public String getpTitle() {
        return pTitle;
    }

    private void setpTitle(String pTitle) {
        this.pTitle = pTitle;
    }

    public DateTime getpDateTime() {
        return pDateTime;
    }

    private void setpDateTime(DateTime pDateTime) {
        this.pDateTime = pDateTime;
    }

    public String getpHandle() {
        return pHandle;
    }

    private void setpHandle(String pHandle) {
        this.pHandle = pHandle;
    }

    public Integer getpId() {
        return pId;
    }

    private void setpId(Integer pId) {
        this.pId = pId;
    }

    public String getpBody() {
        return pBody;
    }

    private void setpBody(String pBody) {
        this.pBody = pBody;
    }

    public ConcurrentMap<String, BufferedImage> getpImages() {
        return pImages;
    }

    private void setpImages(ConcurrentMap<String, BufferedImage> pImages) {
        this.pImages = pImages;
    }

    public List<String> getpLinks() {
        return pLinks;
    }

    private void setpLinks(List<String> pLinks) {
        this.pLinks.addAll(pLinks);
    }

    public ConcurrentMap<String, Integer> getPostPoints() {
        return postPoints;
    }

    private void setPostPoints(ConcurrentMap<String, Integer> postPoints) {
        this.postPoints.putAll(postPoints);
    }

    /* public static List<String> getForumPage() throws IOException {
        String clPoWo = "https://forums.craigslist.org/?act=DF&forumID=20&areaID=9";
        String document = Jsoup.connect(clPoWo).timeout(10000).get().getElementsByClass("threads").html();
        List<String> postRows = new ArrayList<String>(Arrays.asList(document.split("<br />")));
        return sanitizePostRows(postRows);
    }*/

    public void postRowWorker(Element postRowElement, Document postRowDoc, ForumPost forumPost) {
        forumPost.setPostPoints(pointsHandler(postRowElement));
        ConcurrentMap<String, String> postLocations = locationHandler(postRowDoc, postRowElement);
        forumPost.setpLocation(postLocations);
        forumPost.setpId(idHandler(postRowDoc));
    }

    public void postBodyWorker(ForumPost forumPost) throws IOException {
        String clPoWoPostBody = "https://forums.craigslist.org/?act=Q&ID=";
        clPoWoPostBody = clPoWoPostBody.concat(forumPost.getpId().toString());
        Document doc = Jsoup.connect(clPoWoPostBody).timeout(10000).get();
        Elements bodyContent = doc.getElementsByClass("quote");

        // Set title.
        forumPost.setpTitle(doc.select("span:eq(0)").text());

        // Set handle.
        forumPost.setpHandle(doc.select("div#titleLine, b").get(2).text());

        // Settings date time. Sometimes I pick things up I should, and the string needs cleaning.
        forumPost.setpDateTime(dateTimeHandler(doc));

        // Setting body
        if (!bodyContent.select("span.fbod").text().isEmpty()) {
            forumPost.setpBody(bodyContent.select("span.fbod").text());
        } else forumPost.setpBody(null);

        // Storing links independently in a list
        if (bodyContent.select("span.fbod").select("[href]").hasText()) {
            forumPost.setpLinks(linkHandler(bodyContent));
        }

        // Grabbing and setting images
        if (imgURLHandler(bodyContent).size() > 0) {
            System.out.println("Image download start: " + new DateTime());
            try {
                forumPost.setpImages(postImageHandler(imgURLHandler(bodyContent)));
            } catch (Exception e) {
                e.printStackTrace();
            }
            System.out.println("End download: " + new DateTime());
        }
    }

    // Grab and return post id.
    private Integer idHandler(Document postRowDoc) {
        Element postIdElement = postRowDoc.select("a").first();
        String postIdString = postIdElement.attr("href").replace("?act=Q&ID=", "");
        return Integer.parseInt(postIdString);
    }

    // Grab and return post links.
    private List<String> linkHandler(Elements bodyContent) {
        List<String> postLinks = new ArrayList<String>();
        Elements postLinksElements = bodyContent.select("span.fbod").select("[href]");
        for (Element link : postLinksElements) {
            String links = link.select("a[href]").text();
            postLinks.add(links);
        }
        return postLinks;
    }

    // grab and return image links from post.
    private List<String> imgURLHandler(Elements bodyContent) {
        List<String> postImageURLs = new ArrayList<String>();
        Elements postImageElements = bodyContent.select("span.fbod").select("img");
        for (Element img : postImageElements) {
            String imgUrl = img.select("img").attr("src");
            postImageURLs.add(imgUrl);
        }
        return postImageURLs;
    }

    // Parse post points and return hashmap of positive/negative points.
    private ConcurrentMap<String, Integer> pointsHandler(Element postRowElement){
        ConcurrentMap<String, Integer> postPoints = new ConcurrentHashMap<String, Integer>();
        if (postRowElement.select("font > b").hasText()) {
            String pointString;
            pointString = postRowElement.select("font > b").text().replace("+", "");
            Integer pointInt = Integer.parseInt(pointString);
            postPoints.put("posPoints", pointInt);
        }
        if (postRowElement.select("em").hasText()) {
            String pointString;
            pointString = postRowElement.select("em:contains(-)").text().replace("-", "");
            Integer pointInt = Integer.parseInt(pointString);
            postPoints.put("negPoints", pointInt);
        }
        return postPoints;
    }

    // Handles parsing the datetime string and returns a properly formatted datetime to the caller.
    private DateTime dateTimeHandler(Document doc) {
        String postDateTime = doc.select("font").text();
        if (postDateTime.length() != 11) {
            postDateTime = postDateTimeCleanup(doc, postDateTime);
            return convertPostDateTime(postDateTime);
        }
        return convertPostDateTime(postDateTime);
    }

    // Convert the postDateTime string to datetime compatible with MySQL datetime field.
    private DateTime convertPostDateTime(String postDateTimeString) {
        if (postDateTimeString.length() > 11) {
            postDateTimeString = postDateTimeString.substring((postDateTimeString.length() - 11), postDateTimeString.length());
        }
        int currentYear = Calendar.getInstance().get(Calendar.YEAR);
        DateTimeFormatter clFormat = DateTimeFormat.forPattern("MM/dd HH:mm");
        return DateTime.parse(postDateTimeString, clFormat).withYear(currentYear);
    }

    // Method to clean up postDateTime strings that are not correct. We can fix strings that are too long, but too
    // short a postDateTime means we can't provide one, so a default should be returned.
    private String postDateTimeCleanup(Document doc, String postDateTime) {
        String newPostDateTime = null;
        if (postDateTime.length() > 11) {
            System.out.println("WARNING: postDateTime STRING IS LONGER THAN 11 CHARS: " + postDateTime);
            System.out.println("Handle causing issue: " + doc.select("div#titleLine, b").get(2).text());
            int dateSep = postDateTime.indexOf("/");
            if (dateSep > 2) {
                newPostDateTime = postDateTime.substring((postDateTime.length() - 11), 13);
            }
            System.out.println(postDateTime);
        } else {
            System.out.println("WARNING: postDateTime STRING IS SHORTER THAN 14 CHARS: " + postDateTime);
            System.out.println("Handle causing issue: " + doc.select("div#titleLine, b").get(2).text());
            newPostDateTime = new DateTime().toString(); // Returning a datetime to avoid a break.
        }
        return newPostDateTime;
    }

    // Image handler method. Downloads images, returns a hashmap of filename(s) and image(s).
    private ConcurrentMap<String, BufferedImage> postImageHandler(List<String> postImgUrls) {
        ConcurrentMap<String, BufferedImage> postImages = new ConcurrentHashMap<String, BufferedImage>();
        String timestamp = timeStamper();
        File imgFolder = new File(timestamp);
        new File("img/" + imgFolder.toString()).mkdirs();
        for (String imgUrlString : postImgUrls) {
            try {
                URL imgUrl = new URL(imgUrlString);
                String filename = FilenameUtils.getName(imgUrl.toString());
                InputStream inputImgStream = imgUrl.openStream();
                BufferedImage bfrImg = ImageIO.read(inputImgStream);
                /*String mimeType;
                // This if statement does not work. mimeType returns null. In observed cases,
                if (!FilenameUtils.isExtension(filename, postData.getImgMimeList())) {
                    mimeType = URLConnection.guessContentTypeFromStream(inputImgStream);
                    filename.concat("." + mimeType);
                }*/
                OutputStream outputImgStream = new FileOutputStream("img/" + imgFolder.toString() + "/" + filename);
                byte[] buffer = new byte[2048];
                int length = 0;
                while ((length = inputImgStream.read(buffer)) != -1) {
                    outputImgStream.write(buffer, 0, length);
                }
                inputImgStream.close();
                outputImgStream.close();
                postImages.put(filename, bfrImg);
            } catch (IOException ioException) {
                System.out.println("There was a problem with the image handling logic.");
                ioException.printStackTrace();
            } catch (HTTPException e) {
                e.getStatusCode();
            }
        }
        return postImages;
    }

    // This method parses out location data.
    private ConcurrentMap<String, String> locationHandler(Document postRowDoc, Element postRowElement) {
        ConcurrentMap<String, String> postLocation = new ConcurrentHashMap<String, String>();
        if (postRowDoc.select("span[title]").hasAttr("title")) {
            postLocation.put("locationLong", postRowDoc.select("span[title]").attr("title"));
            postLocation.put("locationShort", postRowDoc.select("span[title]").text());
        } else {
            // The following loop i used to obtain the text string for responses, which is a string as opposed
            // to an element.
            Integer tNodeIndex = 0;
            for (Node tNode : postRowElement.childNodes()) {
                if (tNode instanceof TextNode) {
                    if (tNodeIndex == 0) {
                        postLocation.put("locationResponse", tNode.toString());
                        break;
                    }
                    tNodeIndex++;
                }
            }
        }
        return postLocation;
    }

    private String timeStamper() {
        DateTime dt = new DateTime();
        DateTimeFormatter fmt = DateTimeFormat.forPattern("yyyyMMddHHmmssSSS");
        return fmt.print(dt);
    }
}

[Stephan van Hulst]

Sean, I doubt many people are going to look at your code to find out what you want.

All we know is you have some code, and you want to make it multi-threaded. Please give us some more background on what your application is supposed to do, what it's doing now, and where you're running into troubles.

[Sean Funk]

My apologies, I should have been more clear :-)

The point I want to multithread is where I hand off a 'postRow' to a set of methods for processing. We always have a variable number of postRow Strings inside the postRows arraylist. So roughly, this section of code:

                for (String postRow : postRows) {
                    ForumPost forumPost = new ForumPost();
                    Document postRowDoc = Jsoup.parse(postRow);
                    Element postRowElement = postRowDoc.body();
                    forumPost.postRowWorker(postRowElement, postRowDoc, forumPost);
                    try {
                        forumPost.postBodyWorker(forumPost);
                    } catch (IOException e) {
                        e.printStackTrace();
                    }
                    forumPosts.add(forumPost);
                    //postRowCount++;
                }
            }
        };

        executorPool.execute(rowThread);

Currently, the code executes, but it appears to execute only one thread - and the thread never dies so the program hangs once all the rows are complete. I"m still messing around with it locally, but i'll update the thread this evening.

To be more clear, the function of the code posted in my second post is this:

- Pings end point, retrieves a page.
- Passes a certain portion of that page into an array list, separated by <br> (individual posts (or rows) into it's own slot in the list)
- Iterate through that list, grabbing data from each line, as well as creating another get request for each individual post body, whose sourceiD is in each post row (thus the need to process each row, then each body)

At the point of processing the individual rows, and subsequently the post bodies, I would like that to spawn multiple threads. Each post row and body are unique and tied only to one another, not to other post rows or bodies - so my thinking is that each thread can operate independently once it's generated, while waiting for the last thread to finish and then program can end. It's at that point that I would hand the data off for a batch insertion into a mysql db, but I haven't gotten that far yet. :-)

[Stephan van Hulst]

Sorry, I forgot to welcome you to the Ranch. Welcome Sean!

So what you want is to retrieve message headers from a page, then concurrently retrieve each message body and process that in some way? Just pass each header to a task that you submit to the thread pool.

[Stephan van Hulst]

I rewrote some of your code:

package com.blah.dev;

import java.io.*;
import java.net.*;
import java.util.*;
import java.util.concurrent.*;

import org.joda.time.DateTimeZone;
import org.joda.time.Instant;
import org.joda.time.YearMonth;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;

import org.jsoup.Jsoup;
import org.jsoup.nodes.Element;
import org.jsoup.nodes.Node;
import org.jsoup.parser.Parser;
import org.jsoup.select.Elements;

final class ForumReader {
  
  private static final DateTimeZone
    SERVER_TIME_ZONE = DateTimeZone.forOffsetHours(-8);
  
  private static final DateTimeFormatter
    SERVER_FORMAT = DateTimeFormat.forPattern("MM/dd HH:mm")
      .withDefaultYear(new YearMonth(SERVER_TIME_ZONE).getYear())
      .withZone(SERVER_TIME_ZONE);
  
  private final URL url;
  
  ForumReader(URL url) {
    Objects.requireNonNull(url);
    this.url = url;
  }
  
  List<Header> readHeaders() throws IOException {
    List<Header> headers = new ArrayList<>();
    Map<Integer, Header> levels = new HashMap<>();
    
    Element threads = Jsoup.connect(url.toString()).get().select(".threads").first();
    for (Element e: threads.select("a")) {
      Element fe = e.nextElementSibling().nextElementSibling();
      
      Header.Builder b = new Header.Builder()
        .setSubject (extractSubject(e))
        .setUser    (extractUser   (e))
        .setInstant (extractInstant(fe))
        .setLink    (extractLink   (e))
        .setPlus    (extractPlus   (fe))
        .setMinus   (extractMinus  (fe));
      
      extractFeatures(fe).stream().forEach(b::addFeature);
      
      int indents = extractIndents(e);
      if (indents == 0)
        b.setLocation(extractLocation(e));
      else
        b.setInReplyTo(levels.get(indents -1));
      
      Header header = b.createHeader();
      headers.add(header);
      levels.put(indents, header);
    }    
    
    return headers;
  }

  private static String extractSubject(Element e) {
    return Parser.unescapeEntities(e.select("b").first().html(), true);
  }

  private static String extractUser(Element e) {
    StringBuilder b = new StringBuilder();
    for (Node node: e.nextElementSibling().childNodes()) {
      if (node.childNodeSize() > 0)
        b.append('I');
      else
        b.append(node.outerHtml());
    }
      
    return b.toString();
  }

  private static Instant extractInstant(Element fe) {
    return Instant.parse(fe.html().substring(0, 11), SERVER_FORMAT);
  }

  private static String extractLink(Element e) {
    return e.attr("href");
  }

  private static int extractMinus(Element fe) {
    Element minus = fe.select("em").first();
    if (minus == null) return 0;
    
    return Integer.parseInt(minus.html());
  }

  private static int extractPlus(Element fe) {
    Element plus = fe.select("b").first();
    if (plus == null) return 0;
    
    return Integer.parseInt(plus.html());
  }

  private static Set<Feature> extractFeatures(Element fe) {
    Set<Feature> features = EnumSet.noneOf(Feature.class);
    fe.select(".R").stream().forEach(e -> {
      switch (e.html()) {
        case "pic"  : features.add(Feature.PIC) ; break;
        case "link" : features.add(Feature.LINK); break;
      }
    });
    
    return features;
  }

  private static int extractIndents(Element e) {
    Scanner scanner = new Scanner(e.previousSibling().outerHtml());
    int indents = 0;
    while (scanner.findWithinHorizon(": . .", 0) != null)
      indents++;
      
    return indents;
  }

  private static Location extractLocation(Element e) {
    Element le = e.previousElementSibling();
    Elements les = le.select("span");
    if (!les.isEmpty())
      le = les.first();
    
    String locationName = le.attr("title");
    String abbreviation = le.html();
    return new Location(locationName, abbreviation);
  }
  
  public static void main(String... args) throws Exception {
    URL context = new URL("https://forums.craigslist.org");
    URL forum = new URL(context, "?act=DF&forumID=20&areaID=9");
    List<Header> headers = new ForumReader(forum).readHeaders();
    printHeaderTable(headers);
    
    ExecutorService executor = Executors.newCachedThreadPool();
    
    try {
      for (Header header: headers)
        executor.submit(new BodyReader(header, context));
    }
    
    finally {
      executor.shutdown();
    }    
  }
  
  private static void printHeaderTable(Collection<? extends Header> headers) {
    for (Header header: headers) {
      StringBuilder b = new StringBuilder();
      int level = header.level();
      
      b.append(DateTimeFormat.shortDateTime().print(header.instant()));
      b.append(String.format("\t%-24s", '<' + header.user() +'>'));
      
      if (level == 0)
        b.append(header.location().abbreviation());
      
      for (int i = 0; i <= level; i++)
        b.append('\t');
      
      b.append(header.subject());
      System.out.println(b);
    }
  }
}

package com.blah.dev;

import java.net.*;

final class BodyReader implements Runnable {
  
  private final Header header;
  private final URL url;
  
  BodyReader(Header header, URL context) throws MalformedURLException {
    this.header = header;
    this.url = new URL(context, header.link());
  }

  @Override
  public void run() {
    //
  }
}

package com.blah.dev;

import java.util.*;

import org.joda.time.Instant;

final class Header {
  
  private final String subject, user, link;
  private final Location location;
  private final Instant instant;
  private final int plus, minus;
  private final Set<Feature> features;
  private final Header inReplyTo;
  
  private Header(Builder b) {
    this.subject   = b.subject;
    this.user      = b.user;
    this.location  = b.location;
    this.instant   = b.instant;
    this.link      = b.link;
    this.plus      = b.plus;
    this.minus     = b.minus;
    this.features  = Collections.unmodifiableSet(EnumSet.copyOf(b.features));
    this.inReplyTo = b.inReplyTo;
  }
  
  String subject() {
    return subject;
  }
  
  String user() {
    return user;
  }
  
  Location location() {
    if (inReplyTo != null)
      return inReplyTo.location();
    
    else
      return location;
  }
  
  Instant instant() {
    return instant;
  }
  
  String link() {
    return link;
  }
  
  int plus() {
    return plus;
  }
  
  int minus() {
    return minus;
  }
  
  Set<Feature> features() {
    return features;
  }
  
  Header inReplyTo() {
    return inReplyTo;
  }
  
  int level() {
    if (inReplyTo == null)
      return 0;
    
    return inReplyTo.level() +1;
  }
  
  static final class Builder {
    
    private String subject, user, link;
    private Location location;
    private Instant instant;
    private int plus, minus;
    private final EnumSet<Feature> features;
    private Header inReplyTo;
    
    Builder() {
      subject  = "";
      user     = "";
      location = new Location("", "");
      instant  = Instant.now();
      features = EnumSet.noneOf(Feature.class);
    }
    
    Header createHeader() {
      return new Header(this);
    }
    
    Builder setSubject(String subject) {
      Objects.requireNonNull(subject);
      this.subject = subject;
      return this;
    }
    
    Builder setUser(String user) {
      Objects.requireNonNull(user);
      this.user = user;
      return this;
    }
    
    Builder setLocation(Location location) {
      Objects.requireNonNull(location);
      this.location = location;
      return this;
    }
    
    Builder setInstant(Instant instant) {
      Objects.requireNonNull(instant);
      this.instant = instant;
      return this;
    }
    
    Builder setLink(String link) {
      this.link = link;
      return this;
    }
    
    Builder setPlus(int points) {
      if (points < 0) throw new IllegalArgumentException();
      this.plus = points;
      return this;
    }
    
    Builder setMinus(int points) {
      if (points > 0) throw new IllegalArgumentException();
      this.minus = points;
      return this;
    }
    
    Builder addFeature(Feature feature) {
      Objects.requireNonNull(feature);
      features.add(feature);
      return this;
    }
    
    Builder removeFeature(Feature feature) {
      features.remove(feature);
      return this;
    }
    
    Builder setInReplyTo(Header header) {
      inReplyTo = header;      
      return this;
    }
  }
}

package com.blah.dev;

import java.util.*;

final class Location {
  
  private final String name, abbreviation;
  
  Location(String name, String abbreviation) {
    Objects.requireNonNull(name);
    Objects.requireNonNull(abbreviation);
    
    this.name = name;
    this.abbreviation = abbreviation;
  }
  
  String name() {
    return name;
  }
  
  String abbreviation() {
    return abbreviation;
  }
}

package com.blah.dev;

enum Feature {
  PIC, LINK;
}

[Stephan van Hulst]

I think the Header class warrants a closer look.

It's immutable, since that will avoid a lot of problems, especially with concurrency, or when using it as the key in maps. Since it's really annoying to have a constructor that takes a bazillion of parameters, I added a Builder class that will easily let you set various attributes, and then finally you can construct an immutable header out of it. Another addition is that it has a reference to a header it's in reply to.

Note that you shouldn't store points on a timeline as DateTime objects, since DateTimes don't represent moments in time. They represent dates and times on a calendar, which are mostly useful for manipulating data that is meaningful in that particular calendar.
Instead, you should use Instants. They are calendar-agnostic, and represent a unique moment in time, wherever you are in the universe.

[Sean Funk]

...

[Winston Gutkowski]

Sean Funk wrote:Wow!
I'm walking through/debugging the code now - if I'm correct, the code doesn't actually get the post body (which is absolutely OK - I can work from there!)? It executes perfectly. There are several implementations that will take me a little time to work through that you've used here (main(String... args) for example) Good thing I have the week off, I'll be digesting this for a little bit. :-)

Could I suggest then that the main reason is that you've only thought through one way of doing this?

And I say that without, I admit, having looked in detail at your code.

But your questions are all about "how". How do I do [this]? Why is [this] going wrong? - which suggest that you've already decided on an implementation before you know whether it's the right one.

The very fact that you've decided that "multi-threading" is what you want seems like the wrong basis from which to begin a design (or a redesign).
Why do you want to multi-thread? What benefits do you think it will provide? Do you know that it will solve the reasons for redesigning in the first place?

It's perfectly possible that you have good answers to all those questions; but unless we know what they are, all we can work with is the pile of code you submitted, and your assumption that the "change to multi-threading" is required.

Personally, there are two things I try to avoid at all costs when I'm programming: Reflection, and multi-threading.
The first because it seems wonderful, and usually isn't; the latter because the second you enter into a multi-threaded world you take on a whole new set of problems - namely: every gateway to every variable or action in your class becomes a potential "leak" or "anomaly" (ie, it basically doesn't behave the way you expect it to).

Secondly: multi-threaded programs are usually very difficult to test. Properly.

Databases have to deal with multi-threading because ... well ... that's what they were designed for; and it's also why they have teams of people to deal with the results and to test every change to blue heat.

So, first question: given the above, do you still think that multi-threading is the only solution?

Second question: Why? (and please be specific)

Multi-threading can be the right answer, and Java provides a lot of support for it; but IMO you need to really want to do it, and it should be as minimal as possible in all cases.

Sorry if it isn't the answer you wanted, or doesn't help; but it's something (as you may have gathered) that I feel quite strongly about.

Winston

[Sean Funk]

Wow, I apologize for asking this question here. If any mods are available, please move this to the beginner forum. I didn't think posting some code and questions would elicit a response like that.

[Stephan van Hulst]

I wouldn't say this was a beginner question, and don't worry about Winston's response; text doesn't convey emotion well, and he raises some great points, take them to heart. You don't have anything to apologize for, you were stuck on something, and now you have food for thought.