patternjavaMinor
Getting the latest hash from a file
Viewed 0 times
thelatestfilegettinghashfrom
Problem
I'm trying to write clean code, but I'm not so good with exception handling.
How can I improve this method? Or is it good enough already?
This method gets a hash from a file.
How can I improve this method? Or is it good enough already?
This method gets a hash from a file.
public String getLatestHash() {
String hash = "none";
URL packURL = null;
try {
packURL = new URL(ConfigManager.get("play_url") + "/" + HTMLParser.getPackName());
} catch (MalformedURLException e) {
logger.severe("invalid URL!");
logger.severe(e.getMessage());
}
try (InputStream inputStream = packURL.openConnection().getInputStream()) {
hash = DigestUtils.md5Hex(inputStream);
} catch (IOException e) {
e.printStackTrace();
}
return hash;
}Solution
Why do you use
And when you get a
You define
Why is the default hash
Aside from that, though, it looks about as good as it'll get. The multiple separate try/catch blocks are messy, but as far as I can tell that's the best way to do it.
logger.severe(...) in your MalformedURLException, but in your IOException you use e.printStackTrace()? Pick one and stick with it; this can include, for example, totally removing the printStackTrace() and replacing it with a comment that explains that you don't need to log it because [reason]. However, I'd suggest logging everything. It's a lot easier to find bugs if you log everything.try (InputStream inputStream = packURL.openConnection().getInputStream()) {And when you get a
MalformedURLException in the try/catch before, your user is going to receive a very confusing NullPointerException instead of a more palatable (possibly custom) exception that means something in context. You can always catch (NullPointerException e) { throw /your exception here/; } to 'change' the exception being thrown (though be sure to use better indenting)You define
packURL right before you use it, but you define hash way at the beginning. Be consistent -- move String hash = "none"; to the line just before the relevant try.Why is the default hash
"none"? Wouldn't an empty string make more sense?Aside from that, though, it looks about as good as it'll get. The multiple separate try/catch blocks are messy, but as far as I can tell that's the best way to do it.
Code Snippets
try (InputStream inputStream = packURL.openConnection().getInputStream()) {Context
StackExchange Code Review Q#82510, answer score: 3
Revisions (0)
No revisions yet.