Return to Answer

"Cleaned up" code

Questions

1. Collection vs Elements for the parameter of extractWeeks: does this again relate to “use the broadest type of collection possible”?

To be honest, it wasn't a conscious choice since I let the IDE perform Extract Method, but generally, yes. Elements is a type of Collection<Element>, but none of it's features are needed in extractWeeks so you might as well use Collection<Element> to make the method more broadly applicable (even though you might not need it).

2. static member functions vs non-static: I’m definitely going to look into this more myself but I couldn’t help getting confused over why certain functions (like extractWeeks) were static, but others (like 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?

Methods can't be static if they use members of their class. Since weeklyResultUrl uses the fields urlPrefix and urlSuffix, it cannot be static. I could declare all methods none-static, but declaring a method static has a few advantages to the reader and to the programmer:

When calling a static method, you can be sure that it does not modify the instance state. Likewise, when inside a static method, you are not able to modify the instance state. Both of these decrease the mental load when reading and writing code.

Also, since a static clearly doesn't require an instance to function, you are able to call a public static method without an instance from outside the class.

3. The 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?), and if so would the caller have to rethrow them?

Yes, technically you're right. I don't think either of the Exceptions you suggested are quite what you'd want. I would only ever throw IllegalArgumentExcpetion if you pass an invalid argument to a method. I would assume that you could extract the numbers from &rankRange=0-100 and add them as an argument to the method. Then IAE might be more applicable.

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

But NPE definitely doesn't fit. Only ever throw an NPE if something is null when it shouldn't be.

Nobody expects you to do this right of the bad as a novice, but it is something you can strive towards. Notice that the code got longer, which many people think is a bad thing. It isn't. The fact that every method is no longer than four lines makes each methods purpose blindingly obvious, especially if you use meaningful names.

Nobody expects you to do this right of the bat as a novice, but it is something you can strive towards. Notice that the code got longer, which many people think is a bad thing. It isn't. The fact that every method is no longer than four lines makes each methods purpose blindingly obvious, especially if you use meaningful names.