Webscraping tennis data 1.1

Question

I incorporated the substantial changes suggested in my previous question that involved building a web-scraper for gathering tennis data.

The improved code is shown below:

Scraper class:

package scraper;

import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
import org.jsoup.nodes.Element;
import org.jsoup.select.Elements;

import java.io.IOException;
import java.time.Duration;
import java.util.*;
import java.util.stream.Collectors;

public class Scraper {
    private final String urlPrefix;
    private final String urlSuffix;
    private final Duration timeout;

    public Scraper(final String urlPrefix, final String urlSuffix, final Duration timeout) {
        this.urlPrefix = urlPrefix;
        this.urlSuffix = urlSuffix;
        this.timeout = timeout;
    }

    private List<WeeklyResult> scrape() throws ScraperException {
        final List<String> weeks = loadWeeks();

        return loadResults(weeks);
    }

    private List<String> loadWeeks() throws ScraperException {
        final Document document = loadDocument(urlPrefix);
        final Elements elements = selectRankingWeeksElements(document);
        final List<String> weeks = extractWeeks(elements);

        return noEmptyElseThrow(weeks);
    }

    private Document loadDocument(final String url) throws ScraperException {
        try {
            return Jsoup.connect(url).timeout((int) timeout.toMillis()).get();
        } catch (IOException e) {
            throw new ScraperException("Error loading ATP website: " + e.toString());
        }
    }

    private static Elements selectRankingWeeksElements(final Document document) {
        // extract ranking weeks from the dropdown menu
        final Elements result = document.getElementsByAttributeValue("data-value", "rankDate")
                .select("ul li");

        Collections.reverse(result);
        return result;
    }

    private static List<String> extractWeeks(final Collection<Element> elements) {
        return elements.stream()
                        .map(Scraper::extractWeek)
                        .collect(Collectors.toList());
    }

    private static List<String> noEmptyElseThrow(final List<String> weeks) throws ScraperException{
        if (weeks.isEmpty()) {
            throw new ScraperException("Please provide a historical time range! Cannot rank otherwise!");
        } else {
            return weeks;
        }
    }

    private List<WeeklyResult> loadResults(final List<String> weeks) throws ScraperException {
        final List<WeeklyResult> result = new ArrayList<>();
        for (String week : weeks) {
            loadWeeklyResult(week).ifPresent(result::add);
        }
        return result;
    }

    private Optional<WeeklyResult> loadWeeklyResult(final String week) throws ScraperException {
        final Document document = loadDocument(weeklyResultUrl(week));
        final Element playerCell = selectPlayerCellElement(document);
        return Optional.ofNullable(playerCell).map(element -> new WeeklyResult(week, element.text()));
    }

    private String weeklyResultUrl (final String week) {
        return urlPrefix+"rankDate="+week+urlSuffix;
    }

    private static Element selectPlayerCellElement(final Document document) {
        return document.getElementsByClass("player-cell").first();
    }

    private static String extractWeek(final Element li) {
        return li.text().replaceAll("\\.", "-");
    }

    public static void main() throws ScraperException {
        final Scraper scraper =
                new Scraper("https://www.atptour.com/en/rankings/singles?", "&rankRange=0-100", Duration.ofSeconds(90));

        List<WeeklyResult> weeklyResults = scraper.scrape();
        System.out.println(weeklyResults);
        for (final WeeklyResult weeklyResult : weeklyResults) {
            System.out.println("Week: " + weeklyResult.getWeek() + " No.1: " + weeklyResult.getPlayerName());
        }
    }
}

WeeklyResult class:

package scraper;

// A POJO that encapsulates a ranking week and the name of the corresponding No.1 player
public class WeeklyResult {
    private final String week;
    private final String playerName;

    public WeeklyResult(final String week, final String playerName) {
        this.week = week;
        this.playerName = playerName;
    }
    public String getWeek() {
        return week;
    }
    public String getPlayerName() {
        return playerName;
    }
}

ScraperException class:

package scraper;

public class ScraperException extends Exception {
    final String message;
    public ScraperException (String message) {
        this.message = message;
    }
    @Override
    public String toString() {
        return this.message;
    }
}

---

I had some follow up questions to ensure I've truly understood Marv's feedback from my previous question:

1. I like the fact that ScraperException can now provide informative messages to handle exceptions, and importantly groups exceptions for all scraper-related logic. The only thing irking me is the try catch block in loadDocument where I wrap the IOException and re-throw it as a ScraperException. I find it to be kind of ugly, to be honest, and was wondering if there was a more efficient method.

2. Expanding on this, and stemming from Marv's suggestion:

There's something to be said about throwing a checked exception, which might be some further reading points as well.

I did some further research; from what I understand, it seems like some degree of boilerplate try-catch code is a necessary evil while wrapping checked exceptions. Would that be correct?

Answer 1

I see you used your custom ScraperException exception in two different modes: the first is below:

private static List<String> noEmptyElseThrow(final List<String> weeks) throws ScraperException{
    if (weeks.isEmpty()) {
         throw new ScraperException("Please provide a historical time range! Cannot rank otherwise!");
    } else {
        return weeks;
    }
}

To handle this situation, it could be better use the core java IllegalArgumentException thrown to indicate that a method has been passed an illegal or inappropriate argument and because it is a subclass of RuntimeException and then an unchecked exception there is no need to declare it in the signature of the method. So you could write :

private static List<String> noEmptyElseThrow(final List<String> weeks) {
    if (weeks.isEmpty()) {
         throw new IllegalArgumentException("Please provide a historical time range! Cannot rank otherwise!");
    } else {
        return weeks;
}

Note: from comments' section you can check @Marv suggested that the illegal argument is not directly connected to the user's actions but to the site structure. An alternative to my solution could be to maintain the old ScraperException with a more significative message like "weeks calendar cannot be empty", hiding all other aspects to the user.

The second mode is this:

private Document loadDocument(final String url) throws ScraperException {
    try {
        return Jsoup.connect(url).timeout((int) timeout.toMillis()).get();
    } catch (IOException e) {
        throw new ScraperException("Error loading ATP website: " + e.toString());
    }
}

Because you are not hiding the true cause of the ScraperException adding the string representation of the IOException to your custom exception, you could use the fact that exceptions are chainable throwing a new IOException in this way :

private Document loadDocument(final String url) throws IOException {
    try {
        return Jsoup.connect(url).timeout((int) timeout.toMillis()).get();
    } catch (IOException e) {
        throw new IOException("Error loading ATP website: ", e);
    }
}

Note: from comments' section you can check @Marv suggested to apply chain of exceptions to the old ScraperException. In this case the old ScraperException class could be rewritten like below:

public class ScraperException extends Exception {
    private final String message;
    
    public ScraperException (String message) {
        this.message = message;
    }

    public ScraperException(String message, Throwable cause) {
        super(cause);
        this.message = message;
    }
    
    @Override
    public String toString() {
        return this.message;
    }
}

The method loadDocument consequently could be rewritten in this way:

private Document loadDocument(final String url) throws ScraperException {
    try {
        return Jsoup.connect(url).timeout((int) timeout.toMillis()).get();
    } catch (IOException e) {
            throw new ScraperException("Error loading ATP website: ", e);
    }
}