Timeline for Throw exception or let code fail
Current License: CC BY-SA 3.0
13 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Feb 15, 2015 at 13:23 | comment | added | InformedA | Leave it alone if it follows the rules and doesn't hurt any. Why all the harassments? | |
| Jan 25, 2015 at 22:35 | comment | added | Jay | @MarcvanLeeuwen RE the potential race condition: True. I find that a more persuasive argument. | |
| Jan 25, 2015 at 22:34 | comment | added | Jay | @MarcvanLeeuwen Why is efficiency not important in the case where there is an error? I don't think that's necessarily true at all. If in the error case the program will display an error message to the user and stop, while in the success case it does a bunch of additional work, it might be relatively less important. But at this level you don't know that. Maybe when there's an error the caller selects a different NAME and tries again, and keeps trying different names until if finds one that's a success -- for example in a function that's trying to assign a unique name. | |
| Jan 25, 2015 at 15:35 | comment | added | Marc van Leeuwen |
@CompuChip: The relative expense of Contains.Key and Resources.Load is of no importance to my consideration. The call to Contains.Key will only prevent the call to Resources.Load in the erroneous case that the same material is loaded more than once; in that execution path there is an error stop, so efficiency is not important. However when all is well, both calls will be needed in the code proposed.
|
|
| Jan 25, 2015 at 15:10 | comment | added | CompuChip |
@MarcvanLeeuwen this is exactly what I would normally do and in fact was going to suggest before I saw your comment. However I just realised that the Resources.Load call is probably way more expensive than ContainsKey, so depending on how often you expect the addition to fail vs. succeed I might go with the code as written. On a slightly unrelated note, one could opt to have a LoadMaterial and a TryLoadMaterial function.
|
|
| Jan 24, 2015 at 16:58 | comment | added | Matthew |
Doing the extra check can perhaps add a race condition if _Materials can be modified by more than one thread. I like the suggestion that @MarcvanLeeuwen gives.
|
|
| Jan 24, 2015 at 9:02 | comment | added | Marc van Leeuwen |
... Instead consider plunging into _Materials.Add unconditionally, then catch a potential error and in the handler throw a different one. The extra work is now only done in the erroneous cases, the exceptional execution path where you don't really worry about efficiency at all.
|
|
| Jan 24, 2015 at 9:01 | comment | added | Marc van Leeuwen |
Even if the error thrown by _Materials.Add is not the error you want to transmit to the caller, I think this method of relabelling the error is inefficient. For every successful call of LoadMaterial (normal operation) you will have done the same sanity test twice, in LoadMaterial and then again in _Materials.Add. If more layers are wrapped, each employing the same style, you can even have many more times the same test.
|
|
| Jan 24, 2015 at 0:18 | comment | added | jpmc26 | "Explicit is better than implicit." -Zen of Python | |
| Jan 23, 2015 at 8:40 | comment | added | async | "The benefit is that your "custom" exception has an error message that's meaningful to anyone calling this function without knowing how it's implemented (which in the future might be you!)." Top advice. | |
| Jan 23, 2015 at 8:40 | vote | accept | async | ||
| Jan 22, 2015 at 23:53 | comment | added | Frank Hileman | Agree. It is nearly always better to throw an exception for a precondition violation. | |
| Jan 22, 2015 at 23:40 | history | answered | Ixrec | CC BY-SA 3.0 |