Skip to main content
Rollback to Revision 3
Source Link
Mast
  • 13.5k
  • 12
  • 54
  • 119

General Follow-up Questions:

  1. Collection<Element> vs Elements for the parameter of extractWeeks: does this again relate to “use the broadest type of collection possible?
  2. static member functions vs non-static: I’m confused over why certain functions (extractWeeks) were static, but others (weeklyResultUrl) are not static. In both cases, the object doesn’t directly call it, so wouldn’t it make sense to declare all such functions as static?
  3. noEmptyElseThrow strictly isn’t an IOException, is it? Can I throw other exceptions instead (IllegalArgumentExcpetion or NullPointerException, and I’m not sure which is the more suited of the two?); if so would the caller have to rethrow them?

Improving exceptions - ScraperException Questions:

I built a custom exception ScraperException that enriches the exception with an informative message, like so:

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

My idea is to then throw ScraperException to the caller each time, wrapping other exceptions like IOException in it. See the code below:

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 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;
        }
    }

...and so on. For the most part I like this idea, but the single try-catch block in loadDocument is irking me a great deal. Is there a more elegant way to do that?

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

I did some research on this, and it seems like this is generally a drawback of checked exceptions, in that wrapping them will result in boilerplate try-catch clauses. Is this unavoidable?


General Follow-up Questions:

  1. Collection<Element> vs Elements for the parameter of extractWeeks: does this again relate to “use the broadest type of collection possible?
  2. static member functions vs non-static: I’m confused over why certain functions (extractWeeks) were static, but others (weeklyResultUrl) are not static. In both cases, the object doesn’t directly call it, so wouldn’t it make sense to declare all such functions as static?
  3. noEmptyElseThrow strictly isn’t an IOException, is it? Can I throw other exceptions instead (IllegalArgumentExcpetion or NullPointerException, and I’m not sure which is the more suited of the two?); if so would the caller have to rethrow them?

Improving exceptions - ScraperException Questions:

I built a custom exception ScraperException that enriches the exception with an informative message, like so:

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

My idea is to then throw ScraperException to the caller each time, wrapping other exceptions like IOException in it. See the code below:

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 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;
        }
    }

...and so on. For the most part I like this idea, but the single try-catch block in loadDocument is irking me a great deal. Is there a more elegant way to do that?

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

I did some research on this, and it seems like this is generally a drawback of checked exceptions, in that wrapping them will result in boilerplate try-catch clauses. Is this unavoidable?

moved follow ups from comments section to main question, and added follow up question related to the answer
Source Link

General Follow-up Questions:

  1. Collection<Element> vs Elements for the parameter of extractWeeks: does this again relate to “use the broadest type of collection possible?
  2. static member functions vs non-static: I’m confused over why certain functions (extractWeeks) were static, but others (weeklyResultUrl) are not static. In both cases, the object doesn’t directly call it, so wouldn’t it make sense to declare all such functions as static?
  3. noEmptyElseThrow strictly isn’t an IOException, is it? Can I throw other exceptions instead (IllegalArgumentExcpetion or NullPointerException, and I’m not sure which is the more suited of the two?); if so would the caller have to rethrow them?

Improving exceptions - ScraperException Questions:

I built a custom exception ScraperException that enriches the exception with an informative message, like so:

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

My idea is to then throw ScraperException to the caller each time, wrapping other exceptions like IOException in it. See the code below:

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 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;
        }
    }

...and so on. For the most part I like this idea, but the single try-catch block in loadDocument is irking me a great deal. Is there a more elegant way to do that?

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

I did some research on this, and it seems like this is generally a drawback of checked exceptions, in that wrapping them will result in boilerplate try-catch clauses. Is this unavoidable?


General Follow-up Questions:

  1. Collection<Element> vs Elements for the parameter of extractWeeks: does this again relate to “use the broadest type of collection possible?
  2. static member functions vs non-static: I’m confused over why certain functions (extractWeeks) were static, but others (weeklyResultUrl) are not static. In both cases, the object doesn’t directly call it, so wouldn’t it make sense to declare all such functions as static?
  3. noEmptyElseThrow strictly isn’t an IOException, is it? Can I throw other exceptions instead (IllegalArgumentExcpetion or NullPointerException, and I’m not sure which is the more suited of the two?); if so would the caller have to rethrow them?

Improving exceptions - ScraperException Questions:

I built a custom exception ScraperException that enriches the exception with an informative message, like so:

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

My idea is to then throw ScraperException to the caller each time, wrapping other exceptions like IOException in it. See the code below:

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 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;
        }
    }

...and so on. For the most part I like this idea, but the single try-catch block in loadDocument is irking me a great deal. Is there a more elegant way to do that?

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

I did some research on this, and it seems like this is generally a drawback of checked exceptions, in that wrapping them will result in boilerplate try-catch clauses. Is this unavoidable?

Tweeted twitter.com/StackCodeReview/status/1273676881267752960
updated tag
Source Link
Mast
  • 13.5k
  • 12
  • 54
  • 119

Web Scraping Tennis Data in Java Webscraping tennis data

  • Currently my scraper retrieves the No.1s - or so it seems. I haven't tested it apart from just printing it to my console and verifying it that way. One thing I feel is that I can tighten some of the exception handling, but I wasn't sure how what test cases to develop in JUnit for that. Any tips?

  • More importantly, feedback on the code style would be really appreciated! The bulk of my code is in Scraper (duh), but I'm not sure I'm too comfortable with having various static methods. That being said, a sprawling main function is not ideal either, especially when there are separable pieces of the logic that the scraper performs. Does this indicate I need to somehow break the Scraper design into smaller objects? What be a good design practice?

  • Any other feedback, especially related to best practices and idioms in Java would be appreciated (I come from a primarily C & C++ background).

package tennisProject;

import Scraper.Scraper;

public class TennisProject {
    public static void main(String[] args) {
        Scraper.main();
    }
}
```

Web Scraping Tennis Data in Java

  • Currently my scraper retrieves the No.1s - or so it seems. I haven't tested it apart from just printing it to my console and verifying it that way. One thing I feel is that I can tighten some of the exception handling, but I wasn't sure how what test cases to develop in JUnit for that. Any tips?

  • More importantly, feedback on the code style would be really appreciated! The bulk of my code is in Scraper (duh), but I'm not sure I'm too comfortable with having various static methods. That being said, a sprawling main function is not ideal either, especially when there are separable pieces of the logic that the scraper performs. Does this indicate I need to somehow break the Scraper design into smaller objects? What be a good design practice?

  • Any other feedback, especially related to best practices and idioms in Java would be appreciated (I come from a primarily C & C++ background)

package tennisProject;

import Scraper.Scraper;

public class TennisProject {
    public static void main(String[] args) {
        Scraper.main();
    }
}
```

Webscraping tennis data

  • Currently my scraper retrieves the No.1s - or so it seems. I haven't tested it apart from just printing it to my console and verifying it that way. One thing I feel is that I can tighten some of the exception handling, but I wasn't sure how what test cases to develop in JUnit for that. Any tips?

  • More importantly, feedback on the code style would be really appreciated! The bulk of my code is in Scraper (duh), but I'm not sure I'm too comfortable with having various static methods. That being said, a sprawling main function is not ideal either, especially when there are separable pieces of the logic that the scraper performs. Does this indicate I need to somehow break the Scraper design into smaller objects? What be a good design practice?

  • Any other feedback, especially related to best practices and idioms in Java would be appreciated (I come from a primarily C & C++ background).

package tennisProject;

import Scraper.Scraper;

public class TennisProject {
    public static void main(String[] args) {
        Scraper.main();
    }
}
Source Link
Loading