2
\$\begingroup\$

I wrote a program that goes through a webpage and returns matches of regex. I used it on my letterboxd.com account to go through all of my movies (over 900 entries) and then find genres field for each of them in order to get the number of movies I've seen in each genre (that makes 53 pages for getting urls for the movies and 945 movie pages for getting genres). But it took over 30 minutes (the result was correct). A normal webpage takes about a second. So I'd love to get some suggestions for optimizing it.

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import javax.net.ssl.HttpsURLConnection;

/**
 * This program extracts information from html pages.
 * @author Martin Lukáš
 *
 */
public final class HtmlExtractor {

    private final String site;
    private final Pattern p;
    private Matcher matcher;
    private List<String> listOfMatches;
    private String inlineRegex;
    private boolean completed = false;

    /**
     * This constructor creates an object that finds at most one match per line.
     * @param site - your target
     * @param regex - regular expression for finding matches
     */
    public HtmlExtractor(String site, String regex) {
        this.site = site;
        p = Pattern.compile(regex);
        listOfMatches = new ArrayList<>();
    }

    /**
     * This constructor creates an object 
     * that finds the same, as well as multiple matches per line.
     * @param site - your target
     * @param regex - regular expression for finding matches
     * @param inlineRegex - regular expression for dividing a line
     */
    public HtmlExtractor(String site, String regex, String inlineRegex) {
        this.site = site;
        p = Pattern.compile(regex);
        listOfMatches = new ArrayList<>();
        this.inlineRegex = inlineRegex;
    }

    public String getSite() {return site;}
    public String getRegex() {return p.pattern();}
    public String getInlineRegex() {return inlineRegex;}

    public List<String> getMatches() {
        if (completed) return listOfMatches;
        else {
            System.out.println("The extraction didn't take place.");
            return null;
        }
    }

    public boolean isComplete() {return completed;};

    public void findMatches() {
        try {
            extractFrom(new URL(site));
        } catch (MalformedURLException e) {
            System.out.println("The url couldn't be resolved.");
        }
        completed = true;
    }

    private void checkLine(String l) {
        matcher = p.matcher(l);
        if (matcher.find()) {
            if (inlineRegex != null) {
                for (String s: l.split(inlineRegex)) {
                    matcher = p.matcher(s);
                    if (matcher.find()) addToList();
                }
            }
        }
    }

    private void addToList() {
        listOfMatches.add(matcher.group(1));
    }

    private void extractFrom(URL u) {
        String line = "";
        if (site.startsWith("https")) {
            try (BufferedReader in = new BufferedReader(
                    new InputStreamReader(
                            ((HttpsURLConnection) u.openConnection()).getInputStream(), 
                            "UTF-8"))) {
                while ((line = in.readLine()) != null) {
                    checkLine(line);
                }
            } catch (IOException e) {
                System.out.println("This url doesn't exist.");
            }
        } else {
            try (BufferedReader in = new BufferedReader(
                    new InputStreamReader(u.openStream()))) {
                while ((line = in.readLine()) != null) {
                    checkLine(line);
                }
            } catch (IOException e) {
                System.out.println("Couldn't connect to the url.");
            }
        }
    }
}


public class Main {

    public static void main(String[] args) {
        HtmlExtractor h = new HtmlExtractor(
                "http://letterboxd.com//film/double-indemnity/", 
                "(?:films/genre/)(.*)(?=/\" class)",
                "box-link"
        );
        h.findMatches();
        for (String s : h.getMatches()) System.out.println(s);
    }
}

The main() method goes through the page for Double Indemnity and returns the genres. Takes about 1.5 s.

My internet connection shouldn't be a problem (10 Mbps).

\$\endgroup\$
  • 1
    \$\begingroup\$ Regex is a simple but inferior solution to a grammar, parser, and lexer. I'd recommend that you look into that. Or use a Java version of Beautiful Soup, the excellent parser from Python: jsoup.org \$\endgroup\$ – duffymo Jul 22 '16 at 17:08
  • \$\begingroup\$ Worth reading if you haven't already stackoverflow.com/a/1732454/3076272. Read all the answers on that page - what you're doing isn't parsing (bad) but scraping (not necessarily as bad). \$\endgroup\$ – Dannnno Jul 22 '16 at 18:33
1
\$\begingroup\$
public String getSite() {return site;}
public String getRegex() {return p.pattern();}
public String getInlineRegex() {return inlineRegex;}

public List<String> getMatches() {
    if (completed) return listOfMatches;
    else {
        System.out.println("The extraction didn't take place.");
        return null;
    }
}

public boolean isComplete() {return completed;};

You ought to format your code properly. This is hard to read for me.

Here's the recommended formatting:

public String getSite() {
    return site;
}

public String getRegex() {
    return p.pattern();
}

public String getInlineRegex() {
    return inlineRegex;
}

public List<String> getMatches() {
    if (completed) {
        return listOfMatches;
    } else {
        System.out.println("The extraction didn't take place.");
        return null;
    }
}

public boolean isComplete() {
    return completed;
}

We're not in the 80's anymore; you have screen space plenty.


    if (matcher.find()) {
        if (inlineRegex != null) {
            for (String s: l.split(inlineRegex)) {
                matcher = p.matcher(s);
                if (matcher.find()) addToList();
            }
        }
    }

You can combine these two nested if statements:

    if (matcher.find() && inlineRegex != null) {
        for (String s: l.split(inlineRegex)) {
            matcher = p.matcher(s);
            if (matcher.find()) {
                addToList();
            }
        }
    }
\$\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.