Timeline for Configuration concept and implementation
Current License: CC BY-SA 3.0
21 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Dec 19, 2015 at 12:20 | history | edited | RubberDuck | CC BY-SA 3.0 |
added 2 characters in body; edited tags
|
| Sep 19, 2015 at 20:30 | vote | accept | Marco Acierno | ||
| Sep 19, 2015 at 17:00 | answer | added | h.j.k. | timeline score: 2 | |
| Jul 23, 2015 at 2:12 | history | tweeted | twitter.com/#!/StackCodeReview/status/624039347763970049 | ||
| Jul 17, 2015 at 18:32 | comment | added | Marco Acierno | @maaartinus You are right, if I want to make the Configuration interface as generic as possible I can't use IOException since the load could not require IO at all. | |
| Jul 17, 2015 at 18:29 | comment | added | maaartinus |
@MarcoAcierno But if you switch to a database, then IOException makes no sense anymore. Inside you code, IOException is fine, but to the outside I'd prefer to present ConfigurationLoadFailedException. I guess, the parser throws its own exception and you can use your own to hide the details.
|
|
| Jul 17, 2015 at 18:24 | comment | added | Marco Acierno | @maaartinus Yeah, that was one of my fear. With IOException the programmer can do something to recover it (right now, my code in case of IOException tries to load default settings (after asking the user if wants to try or not) if it fails too, just close everything with "Unable to launch") But if something worst happens recover to default settings could just hide the bug and make users unhappy to see their settings lost at every launch | |
| Jul 17, 2015 at 18:21 | comment | added | maaartinus |
@MarcoAcierno You should catch IOException or SQLException, or whatever may happen rather than Exception. In some rare cases, you may also catch RuntimeException, but this is usually a sign of an unexpected bug and then it's damn unsure whether anyone can handle it.
|
|
| Jul 17, 2015 at 15:44 | history | edited | ferada | CC BY-SA 3.0 |
Spelling.
|
| Jul 17, 2015 at 15:27 | comment | added | Ismael Miguel | If it will have the reason accessible to the code, then I don't see anything wrong with it. | |
| Jul 17, 2015 at 15:24 | comment | added | Marco Acierno |
But a catch(Exception e) {} (in the configuration.load code) is ugly, and I don't think that the programmer can recover from every situation @IsmaelMiguel (most of the time the catch code will just say "close the application")
|
|
| Jul 17, 2015 at 15:23 | comment | added | Marco Acierno |
Well, the ConfigurationLoadFailedException have the cause (IOException) so you know what went wrong. With a ConfigurationLoadFailedException I will force the configuration programmer to do try {} catch(Exception e) { throw new ConfigurationLoadFailedException(e); } so the methods always fail with ConfigurationLoadFailedException and in the load settings code one can do catch (ConfigurationLoadFailedException e) { Alert("Ops");} to inform the user. If your point was that with a ConfigurationLoadFailedException one will lose information it won't happen
|
|
| Jul 17, 2015 at 15:17 | comment | added | Ismael Miguel |
I'm not sure if you agree or not with me... But, if there were other cases, the best is to show some very detailed information to the programmer. Or show what you have so you can debug. Now, imagine that you are a programmer. And you are trying to read A. But your code crashed before and the handler to A wasn't released and the file was left opened and locked. With an IOException, you know where to look at. With a ConfigurationLoadFailedException, you don't know if it was a bug in your code or another thing
|
|
| Jul 17, 2015 at 15:11 | comment | added | Marco Acierno | @IsmaelMiguel Well, then I think IOException is fine. It will handle most cases where the programmer can do something about it. If there was other cases (like the one you said) then fail is the best thing to do since the programmer can't recover from unknown errors | |
| Jul 17, 2015 at 15:07 | comment | added | Ismael Miguel |
I don't think that will help. Imagine that you try to read/write on /dev/full. It won't tell that it is a IO error: it will tell that it was something. What was it? Who knows?
|
|
| Jul 17, 2015 at 15:05 | comment | added | Marco Acierno | But maybe a Custom exception is better. So every implementation should wrap their exception in this one and who uses it should only handle one case like "ConfigurationLoadFailedException" and "ConfigurationSaveFailedException" because a load can fail if the settings are in the wrong format | |
| Jul 17, 2015 at 15:04 | comment | added | Marco Acierno | @IsmaelMiguel I've added throws IOException to load too since it seems a good way to force who implements Configuration to handle it | |
| Jul 17, 2015 at 15:02 | comment | added | Ismael Miguel |
It is implied. If the save throws, I would expect the load to throw as well.
|
|
| Jul 17, 2015 at 14:58 | comment | added | Marco Acierno | @IsmaelMiguel It can, maybe I should add this possibility too... | |
| Jul 17, 2015 at 14:45 | comment | added | Ismael Miguel |
On your point 2, why can't load throw such exception?
|
|
| Jul 17, 2015 at 14:27 | history | asked | Marco Acierno | CC BY-SA 3.0 |