Having a flag to indicate if we should throw errors
I recently started working at a place with some much older developers (around 50+ years old). They have worked on critical applications dealing with aviation where the system could not go down. As a result the older programmer tends to code this way.
He tends to put a boolean in the objects to indicate if an exception should be thrown or not.
Example
public class AreaCalculator
{
AreaCalculator(bool shouldThrowExceptions) { ... }
CalculateArea(int x, int y)
{
if(x < 0 || y < 0)
{
if(shouldThrowExceptions)
throwException;
else
return 0;
}
}
}
(In our project the method can fail because we are trying to use a network device that can not be present at the time. The area example is just an example of the exception flag)
To me this seems like a code smell. Writing unit tests becomes slightly more complex since you have to test for the exception flag each time. Also, if something goes wrong, wouldn't you want to know right away? Shouldn't it be the caller's responsibility to determine how to continue?
His logic/reasoning is that our program needs to do 1 thing, show data to user. Any other exception that doesn't stop us from doing so should be ignored. I agree they shouldn't be ignored, but should bubble up and be handled by the appropriate person, and not have to deal with flags for that.
Is this a good way of handling exceptions?
c# exceptions code-reviews
add a comment |
I recently started working at a place with some much older developers (around 50+ years old). They have worked on critical applications dealing with aviation where the system could not go down. As a result the older programmer tends to code this way.
He tends to put a boolean in the objects to indicate if an exception should be thrown or not.
Example
public class AreaCalculator
{
AreaCalculator(bool shouldThrowExceptions) { ... }
CalculateArea(int x, int y)
{
if(x < 0 || y < 0)
{
if(shouldThrowExceptions)
throwException;
else
return 0;
}
}
}
(In our project the method can fail because we are trying to use a network device that can not be present at the time. The area example is just an example of the exception flag)
To me this seems like a code smell. Writing unit tests becomes slightly more complex since you have to test for the exception flag each time. Also, if something goes wrong, wouldn't you want to know right away? Shouldn't it be the caller's responsibility to determine how to continue?
His logic/reasoning is that our program needs to do 1 thing, show data to user. Any other exception that doesn't stop us from doing so should be ignored. I agree they shouldn't be ignored, but should bubble up and be handled by the appropriate person, and not have to deal with flags for that.
Is this a good way of handling exceptions?
c# exceptions code-reviews
1
Is the flag ever set to true in the code base?
– mmathis
8 hours ago
I changed your line "I agree they should be ignored," to "I agree they shouldn't be ignored,", as I think that's what you meant. Please revert or edit further if that was not your intent.
– mmathis
7 hours ago
@mmathis Thanks for the corrections. To answer your previous question, this is for a new class in our code, thus there aren't any calls to this part of code yet.
– Nicolas
7 hours ago
2
c# has a convention for this Try-Parse patter. more info: docs.microsoft.com/en-us/dotnet/standard/design-guidelines/… The flag is not match with this pattern.
– Peter
5 hours ago
add a comment |
I recently started working at a place with some much older developers (around 50+ years old). They have worked on critical applications dealing with aviation where the system could not go down. As a result the older programmer tends to code this way.
He tends to put a boolean in the objects to indicate if an exception should be thrown or not.
Example
public class AreaCalculator
{
AreaCalculator(bool shouldThrowExceptions) { ... }
CalculateArea(int x, int y)
{
if(x < 0 || y < 0)
{
if(shouldThrowExceptions)
throwException;
else
return 0;
}
}
}
(In our project the method can fail because we are trying to use a network device that can not be present at the time. The area example is just an example of the exception flag)
To me this seems like a code smell. Writing unit tests becomes slightly more complex since you have to test for the exception flag each time. Also, if something goes wrong, wouldn't you want to know right away? Shouldn't it be the caller's responsibility to determine how to continue?
His logic/reasoning is that our program needs to do 1 thing, show data to user. Any other exception that doesn't stop us from doing so should be ignored. I agree they shouldn't be ignored, but should bubble up and be handled by the appropriate person, and not have to deal with flags for that.
Is this a good way of handling exceptions?
c# exceptions code-reviews
I recently started working at a place with some much older developers (around 50+ years old). They have worked on critical applications dealing with aviation where the system could not go down. As a result the older programmer tends to code this way.
He tends to put a boolean in the objects to indicate if an exception should be thrown or not.
Example
public class AreaCalculator
{
AreaCalculator(bool shouldThrowExceptions) { ... }
CalculateArea(int x, int y)
{
if(x < 0 || y < 0)
{
if(shouldThrowExceptions)
throwException;
else
return 0;
}
}
}
(In our project the method can fail because we are trying to use a network device that can not be present at the time. The area example is just an example of the exception flag)
To me this seems like a code smell. Writing unit tests becomes slightly more complex since you have to test for the exception flag each time. Also, if something goes wrong, wouldn't you want to know right away? Shouldn't it be the caller's responsibility to determine how to continue?
His logic/reasoning is that our program needs to do 1 thing, show data to user. Any other exception that doesn't stop us from doing so should be ignored. I agree they shouldn't be ignored, but should bubble up and be handled by the appropriate person, and not have to deal with flags for that.
Is this a good way of handling exceptions?
c# exceptions code-reviews
c# exceptions code-reviews
edited 4 hours ago
Nicolas
asked 8 hours ago
NicolasNicolas
345
345
1
Is the flag ever set to true in the code base?
– mmathis
8 hours ago
I changed your line "I agree they should be ignored," to "I agree they shouldn't be ignored,", as I think that's what you meant. Please revert or edit further if that was not your intent.
– mmathis
7 hours ago
@mmathis Thanks for the corrections. To answer your previous question, this is for a new class in our code, thus there aren't any calls to this part of code yet.
– Nicolas
7 hours ago
2
c# has a convention for this Try-Parse patter. more info: docs.microsoft.com/en-us/dotnet/standard/design-guidelines/… The flag is not match with this pattern.
– Peter
5 hours ago
add a comment |
1
Is the flag ever set to true in the code base?
– mmathis
8 hours ago
I changed your line "I agree they should be ignored," to "I agree they shouldn't be ignored,", as I think that's what you meant. Please revert or edit further if that was not your intent.
– mmathis
7 hours ago
@mmathis Thanks for the corrections. To answer your previous question, this is for a new class in our code, thus there aren't any calls to this part of code yet.
– Nicolas
7 hours ago
2
c# has a convention for this Try-Parse patter. more info: docs.microsoft.com/en-us/dotnet/standard/design-guidelines/… The flag is not match with this pattern.
– Peter
5 hours ago
1
1
Is the flag ever set to true in the code base?
– mmathis
8 hours ago
Is the flag ever set to true in the code base?
– mmathis
8 hours ago
I changed your line "I agree they should be ignored," to "I agree they shouldn't be ignored,", as I think that's what you meant. Please revert or edit further if that was not your intent.
– mmathis
7 hours ago
I changed your line "I agree they should be ignored," to "I agree they shouldn't be ignored,", as I think that's what you meant. Please revert or edit further if that was not your intent.
– mmathis
7 hours ago
@mmathis Thanks for the corrections. To answer your previous question, this is for a new class in our code, thus there aren't any calls to this part of code yet.
– Nicolas
7 hours ago
@mmathis Thanks for the corrections. To answer your previous question, this is for a new class in our code, thus there aren't any calls to this part of code yet.
– Nicolas
7 hours ago
2
2
c# has a convention for this Try-Parse patter. more info: docs.microsoft.com/en-us/dotnet/standard/design-guidelines/… The flag is not match with this pattern.
– Peter
5 hours ago
c# has a convention for this Try-Parse patter. more info: docs.microsoft.com/en-us/dotnet/standard/design-guidelines/… The flag is not match with this pattern.
– Peter
5 hours ago
add a comment |
5 Answers
5
active
oldest
votes
The problem with this approach is that while exceptions never get thrown (and thus, the application never crashes due to uncaught exceptions), the results returned are not necessarily correct, and the user may never know that there is a problem with the data (or what that problem is and how to correct it).
In order for the results to be correct and meaningful, the calling method has to check the result for special numbers - i.e., specific return values used to denote problems that came up while executing the method. Negative (or zero) numbers being returned for positive-definite quantities (like area) are a prime example of this in older code. If the calling method doesn't know (or forgets!) to check for these special numbers, though, processing can continue without ever realizing a mistake. Data then gets displayed to the user showing an area of 0, which the user knows is incorrect, but they have no indication of what went wrong, where, or why. They then wonder if any of the other values are wrong...
If the exception was thrown, processing would stop, the error would (ideally) be logged and the user may be notified in some way. The user can then fix whatever is wrong and try again. Proper exception handling (and testing!) will ensure that critical applications do not crash or otherwise wind up in an invalid state.
If your worried about negative values change the method signature to uInt x and uInt y
– Jon Raynor
7 hours ago
add a comment |
Is this a good way of handling exceptions?
No, I think this is pretty bad practice. Throwing an exception vs. returning a value is a fundamental change in the API, changing the method's signature, and making the method behave quite differently from an interface perspective.
In general, when we design classes and their APIs, we should consider that
there may be multiple instances of the class with different configurations floating around in the same program at the same time, and,
due to dependency injection and any number of other programming practices, one consuming client may create the objects and hand them to another other use them — so often we have a separation between object creators and object users.
Consider now what the method caller has to do to make use of an instance s/he's been handed, e.g. for calling the calculating method: the caller would have to both check for area being zero as well as catch exceptions — ouch! Testing considerations go not just to the class itself, but to callers' error handling as well...
We should always be making things as easy as possible for the consuming client; this boolean configuration in the constructor that changes the API of an instance method is the opposite of making the consuming client programmer (maybe you or your colleague) fall into the pit of success.
To offer both APIs, you're much better and more normal to either provide two different classes — one that always throws on error and one that always returns 0 on error, or, provide two different methods with a single class. This way the consuming client can easily know exactly how to check for and handle errors.
Using two different classes or two different methods, you can use the IDEs find method users and refactor features, etc.. much more easily since the two use cases are no longer conflated. Code reading, writing, maintenance, reviews, and testing is simpler as well.
On another note, I personally feel that we should not take boolean configuration parameters, where the actual callers all simply pass a constant. Such configuration parameterization conflates two separate use cases for no real benefit.
Take a look at your code base and see if a variable (or non-constant expression) is ever used for the boolean configuration parameter in the constructor! I doubt it.
And further consideration is to ask why computing the area can fail. Best might be to throw in the constructor, if the calculation cannot be made. However, if you don't know whether the calculation can be made until the object is further initialized, then perhaps consider using different classes to differentiate those states (not ready to compute area vs. ready to compute area).
I read that your failure situation is oriented toward remoting, so may not apply; just some food for thought.
Shouldn't it be the caller's responsibility to determine how to continue?
Yes, I agree. It seems premature for the callee to decide that area of 0 is the right answer under error conditions (especially since 0 is a valid area so no way to tell the difference between error and actual 0, though may not apply to your app).
add a comment |
They have worked on critical applications dealing with aviation where the system could not go down. As a result ...
That is an interesting introduction, which gives me the impression the motivation behind this design is to avoid throwing exceptions in some contexts "because the system could go down" then. But if that actually means "in contexts where exceptions are not handled properly" (and so may cause an interruption of the outer process).
So if the program which uses the AreaCalculator
is buggy, you colleague prefers not to have the program "crash early", but to return some wrong value (hoping no one notices it, or noone does something important with it). That is actually masking an error, and to my experience it will sooner or later lead to bugs for which it gets hard to find the root cause.
So ask yourself what is more important - writing a program does not crash under any circumstances, even when it shows wrong data or calculation results? Or telling the users when something went wrong, so they don't get the impression the data or results they see are correct, even if they are not.
add a comment |
Methods either handle exceptions or they don't, there are no need for flags in languages like C#.
public int Method1()
{
...code
return 0;
}
If something goes bad in ...code then that exception will need to be handled by the caller. If no one handles the error, the program will terminate.
public int Method1()
{
try {
...code
}
catch {}
...Handle error
}
return 0;
}
In this case, if something bad happens in ...code, Method1 is handling the problem and the program should proceed.
Where you handle exceptions is up to you. Certainly you can ignore them by catching and doing nothing. But, I would make sure you are only ignoring certain specific types of exceptions that you can expect to occur. Ignoring (exception ex
) is dangerous because some exceptions you do not want to ignore like system exceptions regarding out of memory and such.
add a comment |
Sometimes throwing an exception is not the best method. Not least due to stack unwinding, but sometimes because catching an exception is problematic, particularly along language or interface seams.
The best way to handle this is to return an enriched data-type. This data type has enough state to describe all of the happy paths, and all of the unhappy paths. The point is, if you interact with this function (member/global/otherwise) you will be forced to handle the outcome.
That being said this enriched data-type should not force action. Imagine in your area example something like var area_calc = new AreaCalculator(); var volume = area_calc.CalculateArea(x, y) * z;
. Seems useful volume
should contain the area multiplied by depth - that could be a cube, cylinder, etc...
But what if the area_calc service was down? Then area_calc .CalculateArea(x, y)
returned a rich datatype containing an error. Is it legal to multiply that by z
? Its a good question. You could force users to handle the checking immediately. This does however break up the logic with error handling.
var area_calc = new AreaCalculator();
var area_result = area_calc.CalculateArea(x, y);
if (area_result.bad())
{
//handle unhappy path
}
var volume = area_result.value() * z;
vs
var area_calc = new AreaCalculator();
var volume = area_calc.CalculateArea(x, y) * z;
if (volume.bad())
{
//handle unhappy path
}
The essentially logic is spread over two lines and divided by error handling in the first case, while the second case has all the relevant logic on one line followed by error handling.
In that second case volume
is a rich data type. Its not just a number. This makes storage larger, and volume
will still need to be investigated for an error condition. Additionally volume
might feed other calculations before the user chooses to handle the error, allowing it to manifest in several disparate locations. This might be good, or bad depending on the specifics of the situation.
Alternately volume
could be just a plain data type - just a number, but then what happens to the error condition? It could be that the value implicitly converts if it is in a happy condition. Should it be in an unhappy condition it might return a default/error value (for area 0 or -1 might seem reasonable). Alternately it could throw an exception on this side of the interface/language boundary.
... foo() {
var area_calc = new AreaCalculator();
return area_calc.CalculateArea(x, y) * z;
}
var volume = foo();
if (volume <= 0)
{
//handle error
}
vs.
... foo() {
var area_calc = new AreaCalculator();
return area_calc.CalculateArea(x, y) * z;
}
try { var volume = foo(); }
catch(...)
{
//handle error
}
By passing out a bad, or possibly bad value, it places a lot of onus on the user to validate the data. This is a source of bugs, because as far as the compiler is concerned the return value is a legitimate integer. If something was not checked you'll discover it when things go wrong. The second case mixes the best of both worlds by allowing exceptions to handle unhappy paths, while happy paths follow normal processing. Unfortunately it does force the user to handle exceptions wisely, which is hard.
Just to be clear an Unhappy path is a case unknown to business logic (the domain of exception), failing to validate is a happy path because you know how to handle that by the business rules (the domain of rules).
The ultimate solution would be one which allows all scenarios (within reason).
- The user should be able to query for a bad condition, and handle it immediately
- The user should be able to operate on the enriched type as if the happy path had been followed and propagate the error details.
- The user should be able to extract the happy path value through casting (implicit/explicit as is reasonable), generating an exception for unhappy paths.
- The user should be able to extract the happy path value, or use a default (supplied or not)
Something like:
Rich::value_type value_or_default(Rich&, Rich::value_type default_value = ...);
bool bad(Rich&);
...unhappy path report... bad_state(Rich&);
Rich& assert_not_bad(Rich&);
class Rich
{
public:
typedef ... value_type;
operator value_type() { assert_not_bad(*this); return ...value...; }
operator X(...) { if (bad(*this)) return ...propagate badness to new value...; /*operate and generate new value*/; }
}
//check
if (bad(x))
{
var report = bad_state(x);
//handle error
}
//rethrow
assert_not_bad(x);
var result = (assert_not_bad(x) + 23) / 45;
//propogate
var y = x * 23;
//implicit throw
Rich::value_type val = x;
var val = ((Rich::value_type)x) + 34;
var val2 = static_cast<Rich::value_type>(x) % 3;
//default value
var defaulted = value_or_default(x);
var defaulted_to = value_or_default(x, 55);
add a comment |
Your Answer
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "131"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e) {
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom)) {
StackExchange.using('gps', function() { StackExchange.gps.track('embedded_signup_form.view', { location: 'question_page' }); });
$window.unbind('scroll', onScroll);
}
};
$window.on('scroll', onScroll);
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fsoftwareengineering.stackexchange.com%2fquestions%2f386702%2fhaving-a-flag-to-indicate-if-we-should-throw-errors%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
5 Answers
5
active
oldest
votes
5 Answers
5
active
oldest
votes
active
oldest
votes
active
oldest
votes
The problem with this approach is that while exceptions never get thrown (and thus, the application never crashes due to uncaught exceptions), the results returned are not necessarily correct, and the user may never know that there is a problem with the data (or what that problem is and how to correct it).
In order for the results to be correct and meaningful, the calling method has to check the result for special numbers - i.e., specific return values used to denote problems that came up while executing the method. Negative (or zero) numbers being returned for positive-definite quantities (like area) are a prime example of this in older code. If the calling method doesn't know (or forgets!) to check for these special numbers, though, processing can continue without ever realizing a mistake. Data then gets displayed to the user showing an area of 0, which the user knows is incorrect, but they have no indication of what went wrong, where, or why. They then wonder if any of the other values are wrong...
If the exception was thrown, processing would stop, the error would (ideally) be logged and the user may be notified in some way. The user can then fix whatever is wrong and try again. Proper exception handling (and testing!) will ensure that critical applications do not crash or otherwise wind up in an invalid state.
If your worried about negative values change the method signature to uInt x and uInt y
– Jon Raynor
7 hours ago
add a comment |
The problem with this approach is that while exceptions never get thrown (and thus, the application never crashes due to uncaught exceptions), the results returned are not necessarily correct, and the user may never know that there is a problem with the data (or what that problem is and how to correct it).
In order for the results to be correct and meaningful, the calling method has to check the result for special numbers - i.e., specific return values used to denote problems that came up while executing the method. Negative (or zero) numbers being returned for positive-definite quantities (like area) are a prime example of this in older code. If the calling method doesn't know (or forgets!) to check for these special numbers, though, processing can continue without ever realizing a mistake. Data then gets displayed to the user showing an area of 0, which the user knows is incorrect, but they have no indication of what went wrong, where, or why. They then wonder if any of the other values are wrong...
If the exception was thrown, processing would stop, the error would (ideally) be logged and the user may be notified in some way. The user can then fix whatever is wrong and try again. Proper exception handling (and testing!) will ensure that critical applications do not crash or otherwise wind up in an invalid state.
If your worried about negative values change the method signature to uInt x and uInt y
– Jon Raynor
7 hours ago
add a comment |
The problem with this approach is that while exceptions never get thrown (and thus, the application never crashes due to uncaught exceptions), the results returned are not necessarily correct, and the user may never know that there is a problem with the data (or what that problem is and how to correct it).
In order for the results to be correct and meaningful, the calling method has to check the result for special numbers - i.e., specific return values used to denote problems that came up while executing the method. Negative (or zero) numbers being returned for positive-definite quantities (like area) are a prime example of this in older code. If the calling method doesn't know (or forgets!) to check for these special numbers, though, processing can continue without ever realizing a mistake. Data then gets displayed to the user showing an area of 0, which the user knows is incorrect, but they have no indication of what went wrong, where, or why. They then wonder if any of the other values are wrong...
If the exception was thrown, processing would stop, the error would (ideally) be logged and the user may be notified in some way. The user can then fix whatever is wrong and try again. Proper exception handling (and testing!) will ensure that critical applications do not crash or otherwise wind up in an invalid state.
The problem with this approach is that while exceptions never get thrown (and thus, the application never crashes due to uncaught exceptions), the results returned are not necessarily correct, and the user may never know that there is a problem with the data (or what that problem is and how to correct it).
In order for the results to be correct and meaningful, the calling method has to check the result for special numbers - i.e., specific return values used to denote problems that came up while executing the method. Negative (or zero) numbers being returned for positive-definite quantities (like area) are a prime example of this in older code. If the calling method doesn't know (or forgets!) to check for these special numbers, though, processing can continue without ever realizing a mistake. Data then gets displayed to the user showing an area of 0, which the user knows is incorrect, but they have no indication of what went wrong, where, or why. They then wonder if any of the other values are wrong...
If the exception was thrown, processing would stop, the error would (ideally) be logged and the user may be notified in some way. The user can then fix whatever is wrong and try again. Proper exception handling (and testing!) will ensure that critical applications do not crash or otherwise wind up in an invalid state.
answered 7 hours ago
mmathismmathis
2,083619
2,083619
If your worried about negative values change the method signature to uInt x and uInt y
– Jon Raynor
7 hours ago
add a comment |
If your worried about negative values change the method signature to uInt x and uInt y
– Jon Raynor
7 hours ago
If your worried about negative values change the method signature to uInt x and uInt y
– Jon Raynor
7 hours ago
If your worried about negative values change the method signature to uInt x and uInt y
– Jon Raynor
7 hours ago
add a comment |
Is this a good way of handling exceptions?
No, I think this is pretty bad practice. Throwing an exception vs. returning a value is a fundamental change in the API, changing the method's signature, and making the method behave quite differently from an interface perspective.
In general, when we design classes and their APIs, we should consider that
there may be multiple instances of the class with different configurations floating around in the same program at the same time, and,
due to dependency injection and any number of other programming practices, one consuming client may create the objects and hand them to another other use them — so often we have a separation between object creators and object users.
Consider now what the method caller has to do to make use of an instance s/he's been handed, e.g. for calling the calculating method: the caller would have to both check for area being zero as well as catch exceptions — ouch! Testing considerations go not just to the class itself, but to callers' error handling as well...
We should always be making things as easy as possible for the consuming client; this boolean configuration in the constructor that changes the API of an instance method is the opposite of making the consuming client programmer (maybe you or your colleague) fall into the pit of success.
To offer both APIs, you're much better and more normal to either provide two different classes — one that always throws on error and one that always returns 0 on error, or, provide two different methods with a single class. This way the consuming client can easily know exactly how to check for and handle errors.
Using two different classes or two different methods, you can use the IDEs find method users and refactor features, etc.. much more easily since the two use cases are no longer conflated. Code reading, writing, maintenance, reviews, and testing is simpler as well.
On another note, I personally feel that we should not take boolean configuration parameters, where the actual callers all simply pass a constant. Such configuration parameterization conflates two separate use cases for no real benefit.
Take a look at your code base and see if a variable (or non-constant expression) is ever used for the boolean configuration parameter in the constructor! I doubt it.
And further consideration is to ask why computing the area can fail. Best might be to throw in the constructor, if the calculation cannot be made. However, if you don't know whether the calculation can be made until the object is further initialized, then perhaps consider using different classes to differentiate those states (not ready to compute area vs. ready to compute area).
I read that your failure situation is oriented toward remoting, so may not apply; just some food for thought.
Shouldn't it be the caller's responsibility to determine how to continue?
Yes, I agree. It seems premature for the callee to decide that area of 0 is the right answer under error conditions (especially since 0 is a valid area so no way to tell the difference between error and actual 0, though may not apply to your app).
add a comment |
Is this a good way of handling exceptions?
No, I think this is pretty bad practice. Throwing an exception vs. returning a value is a fundamental change in the API, changing the method's signature, and making the method behave quite differently from an interface perspective.
In general, when we design classes and their APIs, we should consider that
there may be multiple instances of the class with different configurations floating around in the same program at the same time, and,
due to dependency injection and any number of other programming practices, one consuming client may create the objects and hand them to another other use them — so often we have a separation between object creators and object users.
Consider now what the method caller has to do to make use of an instance s/he's been handed, e.g. for calling the calculating method: the caller would have to both check for area being zero as well as catch exceptions — ouch! Testing considerations go not just to the class itself, but to callers' error handling as well...
We should always be making things as easy as possible for the consuming client; this boolean configuration in the constructor that changes the API of an instance method is the opposite of making the consuming client programmer (maybe you or your colleague) fall into the pit of success.
To offer both APIs, you're much better and more normal to either provide two different classes — one that always throws on error and one that always returns 0 on error, or, provide two different methods with a single class. This way the consuming client can easily know exactly how to check for and handle errors.
Using two different classes or two different methods, you can use the IDEs find method users and refactor features, etc.. much more easily since the two use cases are no longer conflated. Code reading, writing, maintenance, reviews, and testing is simpler as well.
On another note, I personally feel that we should not take boolean configuration parameters, where the actual callers all simply pass a constant. Such configuration parameterization conflates two separate use cases for no real benefit.
Take a look at your code base and see if a variable (or non-constant expression) is ever used for the boolean configuration parameter in the constructor! I doubt it.
And further consideration is to ask why computing the area can fail. Best might be to throw in the constructor, if the calculation cannot be made. However, if you don't know whether the calculation can be made until the object is further initialized, then perhaps consider using different classes to differentiate those states (not ready to compute area vs. ready to compute area).
I read that your failure situation is oriented toward remoting, so may not apply; just some food for thought.
Shouldn't it be the caller's responsibility to determine how to continue?
Yes, I agree. It seems premature for the callee to decide that area of 0 is the right answer under error conditions (especially since 0 is a valid area so no way to tell the difference between error and actual 0, though may not apply to your app).
add a comment |
Is this a good way of handling exceptions?
No, I think this is pretty bad practice. Throwing an exception vs. returning a value is a fundamental change in the API, changing the method's signature, and making the method behave quite differently from an interface perspective.
In general, when we design classes and their APIs, we should consider that
there may be multiple instances of the class with different configurations floating around in the same program at the same time, and,
due to dependency injection and any number of other programming practices, one consuming client may create the objects and hand them to another other use them — so often we have a separation between object creators and object users.
Consider now what the method caller has to do to make use of an instance s/he's been handed, e.g. for calling the calculating method: the caller would have to both check for area being zero as well as catch exceptions — ouch! Testing considerations go not just to the class itself, but to callers' error handling as well...
We should always be making things as easy as possible for the consuming client; this boolean configuration in the constructor that changes the API of an instance method is the opposite of making the consuming client programmer (maybe you or your colleague) fall into the pit of success.
To offer both APIs, you're much better and more normal to either provide two different classes — one that always throws on error and one that always returns 0 on error, or, provide two different methods with a single class. This way the consuming client can easily know exactly how to check for and handle errors.
Using two different classes or two different methods, you can use the IDEs find method users and refactor features, etc.. much more easily since the two use cases are no longer conflated. Code reading, writing, maintenance, reviews, and testing is simpler as well.
On another note, I personally feel that we should not take boolean configuration parameters, where the actual callers all simply pass a constant. Such configuration parameterization conflates two separate use cases for no real benefit.
Take a look at your code base and see if a variable (or non-constant expression) is ever used for the boolean configuration parameter in the constructor! I doubt it.
And further consideration is to ask why computing the area can fail. Best might be to throw in the constructor, if the calculation cannot be made. However, if you don't know whether the calculation can be made until the object is further initialized, then perhaps consider using different classes to differentiate those states (not ready to compute area vs. ready to compute area).
I read that your failure situation is oriented toward remoting, so may not apply; just some food for thought.
Shouldn't it be the caller's responsibility to determine how to continue?
Yes, I agree. It seems premature for the callee to decide that area of 0 is the right answer under error conditions (especially since 0 is a valid area so no way to tell the difference between error and actual 0, though may not apply to your app).
Is this a good way of handling exceptions?
No, I think this is pretty bad practice. Throwing an exception vs. returning a value is a fundamental change in the API, changing the method's signature, and making the method behave quite differently from an interface perspective.
In general, when we design classes and their APIs, we should consider that
there may be multiple instances of the class with different configurations floating around in the same program at the same time, and,
due to dependency injection and any number of other programming practices, one consuming client may create the objects and hand them to another other use them — so often we have a separation between object creators and object users.
Consider now what the method caller has to do to make use of an instance s/he's been handed, e.g. for calling the calculating method: the caller would have to both check for area being zero as well as catch exceptions — ouch! Testing considerations go not just to the class itself, but to callers' error handling as well...
We should always be making things as easy as possible for the consuming client; this boolean configuration in the constructor that changes the API of an instance method is the opposite of making the consuming client programmer (maybe you or your colleague) fall into the pit of success.
To offer both APIs, you're much better and more normal to either provide two different classes — one that always throws on error and one that always returns 0 on error, or, provide two different methods with a single class. This way the consuming client can easily know exactly how to check for and handle errors.
Using two different classes or two different methods, you can use the IDEs find method users and refactor features, etc.. much more easily since the two use cases are no longer conflated. Code reading, writing, maintenance, reviews, and testing is simpler as well.
On another note, I personally feel that we should not take boolean configuration parameters, where the actual callers all simply pass a constant. Such configuration parameterization conflates two separate use cases for no real benefit.
Take a look at your code base and see if a variable (or non-constant expression) is ever used for the boolean configuration parameter in the constructor! I doubt it.
And further consideration is to ask why computing the area can fail. Best might be to throw in the constructor, if the calculation cannot be made. However, if you don't know whether the calculation can be made until the object is further initialized, then perhaps consider using different classes to differentiate those states (not ready to compute area vs. ready to compute area).
I read that your failure situation is oriented toward remoting, so may not apply; just some food for thought.
Shouldn't it be the caller's responsibility to determine how to continue?
Yes, I agree. It seems premature for the callee to decide that area of 0 is the right answer under error conditions (especially since 0 is a valid area so no way to tell the difference between error and actual 0, though may not apply to your app).
edited 2 hours ago
answered 6 hours ago
Erik EidtErik Eidt
23.9k43465
23.9k43465
add a comment |
add a comment |
They have worked on critical applications dealing with aviation where the system could not go down. As a result ...
That is an interesting introduction, which gives me the impression the motivation behind this design is to avoid throwing exceptions in some contexts "because the system could go down" then. But if that actually means "in contexts where exceptions are not handled properly" (and so may cause an interruption of the outer process).
So if the program which uses the AreaCalculator
is buggy, you colleague prefers not to have the program "crash early", but to return some wrong value (hoping no one notices it, or noone does something important with it). That is actually masking an error, and to my experience it will sooner or later lead to bugs for which it gets hard to find the root cause.
So ask yourself what is more important - writing a program does not crash under any circumstances, even when it shows wrong data or calculation results? Or telling the users when something went wrong, so they don't get the impression the data or results they see are correct, even if they are not.
add a comment |
They have worked on critical applications dealing with aviation where the system could not go down. As a result ...
That is an interesting introduction, which gives me the impression the motivation behind this design is to avoid throwing exceptions in some contexts "because the system could go down" then. But if that actually means "in contexts where exceptions are not handled properly" (and so may cause an interruption of the outer process).
So if the program which uses the AreaCalculator
is buggy, you colleague prefers not to have the program "crash early", but to return some wrong value (hoping no one notices it, or noone does something important with it). That is actually masking an error, and to my experience it will sooner or later lead to bugs for which it gets hard to find the root cause.
So ask yourself what is more important - writing a program does not crash under any circumstances, even when it shows wrong data or calculation results? Or telling the users when something went wrong, so they don't get the impression the data or results they see are correct, even if they are not.
add a comment |
They have worked on critical applications dealing with aviation where the system could not go down. As a result ...
That is an interesting introduction, which gives me the impression the motivation behind this design is to avoid throwing exceptions in some contexts "because the system could go down" then. But if that actually means "in contexts where exceptions are not handled properly" (and so may cause an interruption of the outer process).
So if the program which uses the AreaCalculator
is buggy, you colleague prefers not to have the program "crash early", but to return some wrong value (hoping no one notices it, or noone does something important with it). That is actually masking an error, and to my experience it will sooner or later lead to bugs for which it gets hard to find the root cause.
So ask yourself what is more important - writing a program does not crash under any circumstances, even when it shows wrong data or calculation results? Or telling the users when something went wrong, so they don't get the impression the data or results they see are correct, even if they are not.
They have worked on critical applications dealing with aviation where the system could not go down. As a result ...
That is an interesting introduction, which gives me the impression the motivation behind this design is to avoid throwing exceptions in some contexts "because the system could go down" then. But if that actually means "in contexts where exceptions are not handled properly" (and so may cause an interruption of the outer process).
So if the program which uses the AreaCalculator
is buggy, you colleague prefers not to have the program "crash early", but to return some wrong value (hoping no one notices it, or noone does something important with it). That is actually masking an error, and to my experience it will sooner or later lead to bugs for which it gets hard to find the root cause.
So ask yourself what is more important - writing a program does not crash under any circumstances, even when it shows wrong data or calculation results? Or telling the users when something went wrong, so they don't get the impression the data or results they see are correct, even if they are not.
edited 5 hours ago
answered 5 hours ago
Doc BrownDoc Brown
132k23245386
132k23245386
add a comment |
add a comment |
Methods either handle exceptions or they don't, there are no need for flags in languages like C#.
public int Method1()
{
...code
return 0;
}
If something goes bad in ...code then that exception will need to be handled by the caller. If no one handles the error, the program will terminate.
public int Method1()
{
try {
...code
}
catch {}
...Handle error
}
return 0;
}
In this case, if something bad happens in ...code, Method1 is handling the problem and the program should proceed.
Where you handle exceptions is up to you. Certainly you can ignore them by catching and doing nothing. But, I would make sure you are only ignoring certain specific types of exceptions that you can expect to occur. Ignoring (exception ex
) is dangerous because some exceptions you do not want to ignore like system exceptions regarding out of memory and such.
add a comment |
Methods either handle exceptions or they don't, there are no need for flags in languages like C#.
public int Method1()
{
...code
return 0;
}
If something goes bad in ...code then that exception will need to be handled by the caller. If no one handles the error, the program will terminate.
public int Method1()
{
try {
...code
}
catch {}
...Handle error
}
return 0;
}
In this case, if something bad happens in ...code, Method1 is handling the problem and the program should proceed.
Where you handle exceptions is up to you. Certainly you can ignore them by catching and doing nothing. But, I would make sure you are only ignoring certain specific types of exceptions that you can expect to occur. Ignoring (exception ex
) is dangerous because some exceptions you do not want to ignore like system exceptions regarding out of memory and such.
add a comment |
Methods either handle exceptions or they don't, there are no need for flags in languages like C#.
public int Method1()
{
...code
return 0;
}
If something goes bad in ...code then that exception will need to be handled by the caller. If no one handles the error, the program will terminate.
public int Method1()
{
try {
...code
}
catch {}
...Handle error
}
return 0;
}
In this case, if something bad happens in ...code, Method1 is handling the problem and the program should proceed.
Where you handle exceptions is up to you. Certainly you can ignore them by catching and doing nothing. But, I would make sure you are only ignoring certain specific types of exceptions that you can expect to occur. Ignoring (exception ex
) is dangerous because some exceptions you do not want to ignore like system exceptions regarding out of memory and such.
Methods either handle exceptions or they don't, there are no need for flags in languages like C#.
public int Method1()
{
...code
return 0;
}
If something goes bad in ...code then that exception will need to be handled by the caller. If no one handles the error, the program will terminate.
public int Method1()
{
try {
...code
}
catch {}
...Handle error
}
return 0;
}
In this case, if something bad happens in ...code, Method1 is handling the problem and the program should proceed.
Where you handle exceptions is up to you. Certainly you can ignore them by catching and doing nothing. But, I would make sure you are only ignoring certain specific types of exceptions that you can expect to occur. Ignoring (exception ex
) is dangerous because some exceptions you do not want to ignore like system exceptions regarding out of memory and such.
answered 7 hours ago
Jon RaynorJon Raynor
8,7931840
8,7931840
add a comment |
add a comment |
Sometimes throwing an exception is not the best method. Not least due to stack unwinding, but sometimes because catching an exception is problematic, particularly along language or interface seams.
The best way to handle this is to return an enriched data-type. This data type has enough state to describe all of the happy paths, and all of the unhappy paths. The point is, if you interact with this function (member/global/otherwise) you will be forced to handle the outcome.
That being said this enriched data-type should not force action. Imagine in your area example something like var area_calc = new AreaCalculator(); var volume = area_calc.CalculateArea(x, y) * z;
. Seems useful volume
should contain the area multiplied by depth - that could be a cube, cylinder, etc...
But what if the area_calc service was down? Then area_calc .CalculateArea(x, y)
returned a rich datatype containing an error. Is it legal to multiply that by z
? Its a good question. You could force users to handle the checking immediately. This does however break up the logic with error handling.
var area_calc = new AreaCalculator();
var area_result = area_calc.CalculateArea(x, y);
if (area_result.bad())
{
//handle unhappy path
}
var volume = area_result.value() * z;
vs
var area_calc = new AreaCalculator();
var volume = area_calc.CalculateArea(x, y) * z;
if (volume.bad())
{
//handle unhappy path
}
The essentially logic is spread over two lines and divided by error handling in the first case, while the second case has all the relevant logic on one line followed by error handling.
In that second case volume
is a rich data type. Its not just a number. This makes storage larger, and volume
will still need to be investigated for an error condition. Additionally volume
might feed other calculations before the user chooses to handle the error, allowing it to manifest in several disparate locations. This might be good, or bad depending on the specifics of the situation.
Alternately volume
could be just a plain data type - just a number, but then what happens to the error condition? It could be that the value implicitly converts if it is in a happy condition. Should it be in an unhappy condition it might return a default/error value (for area 0 or -1 might seem reasonable). Alternately it could throw an exception on this side of the interface/language boundary.
... foo() {
var area_calc = new AreaCalculator();
return area_calc.CalculateArea(x, y) * z;
}
var volume = foo();
if (volume <= 0)
{
//handle error
}
vs.
... foo() {
var area_calc = new AreaCalculator();
return area_calc.CalculateArea(x, y) * z;
}
try { var volume = foo(); }
catch(...)
{
//handle error
}
By passing out a bad, or possibly bad value, it places a lot of onus on the user to validate the data. This is a source of bugs, because as far as the compiler is concerned the return value is a legitimate integer. If something was not checked you'll discover it when things go wrong. The second case mixes the best of both worlds by allowing exceptions to handle unhappy paths, while happy paths follow normal processing. Unfortunately it does force the user to handle exceptions wisely, which is hard.
Just to be clear an Unhappy path is a case unknown to business logic (the domain of exception), failing to validate is a happy path because you know how to handle that by the business rules (the domain of rules).
The ultimate solution would be one which allows all scenarios (within reason).
- The user should be able to query for a bad condition, and handle it immediately
- The user should be able to operate on the enriched type as if the happy path had been followed and propagate the error details.
- The user should be able to extract the happy path value through casting (implicit/explicit as is reasonable), generating an exception for unhappy paths.
- The user should be able to extract the happy path value, or use a default (supplied or not)
Something like:
Rich::value_type value_or_default(Rich&, Rich::value_type default_value = ...);
bool bad(Rich&);
...unhappy path report... bad_state(Rich&);
Rich& assert_not_bad(Rich&);
class Rich
{
public:
typedef ... value_type;
operator value_type() { assert_not_bad(*this); return ...value...; }
operator X(...) { if (bad(*this)) return ...propagate badness to new value...; /*operate and generate new value*/; }
}
//check
if (bad(x))
{
var report = bad_state(x);
//handle error
}
//rethrow
assert_not_bad(x);
var result = (assert_not_bad(x) + 23) / 45;
//propogate
var y = x * 23;
//implicit throw
Rich::value_type val = x;
var val = ((Rich::value_type)x) + 34;
var val2 = static_cast<Rich::value_type>(x) % 3;
//default value
var defaulted = value_or_default(x);
var defaulted_to = value_or_default(x, 55);
add a comment |
Sometimes throwing an exception is not the best method. Not least due to stack unwinding, but sometimes because catching an exception is problematic, particularly along language or interface seams.
The best way to handle this is to return an enriched data-type. This data type has enough state to describe all of the happy paths, and all of the unhappy paths. The point is, if you interact with this function (member/global/otherwise) you will be forced to handle the outcome.
That being said this enriched data-type should not force action. Imagine in your area example something like var area_calc = new AreaCalculator(); var volume = area_calc.CalculateArea(x, y) * z;
. Seems useful volume
should contain the area multiplied by depth - that could be a cube, cylinder, etc...
But what if the area_calc service was down? Then area_calc .CalculateArea(x, y)
returned a rich datatype containing an error. Is it legal to multiply that by z
? Its a good question. You could force users to handle the checking immediately. This does however break up the logic with error handling.
var area_calc = new AreaCalculator();
var area_result = area_calc.CalculateArea(x, y);
if (area_result.bad())
{
//handle unhappy path
}
var volume = area_result.value() * z;
vs
var area_calc = new AreaCalculator();
var volume = area_calc.CalculateArea(x, y) * z;
if (volume.bad())
{
//handle unhappy path
}
The essentially logic is spread over two lines and divided by error handling in the first case, while the second case has all the relevant logic on one line followed by error handling.
In that second case volume
is a rich data type. Its not just a number. This makes storage larger, and volume
will still need to be investigated for an error condition. Additionally volume
might feed other calculations before the user chooses to handle the error, allowing it to manifest in several disparate locations. This might be good, or bad depending on the specifics of the situation.
Alternately volume
could be just a plain data type - just a number, but then what happens to the error condition? It could be that the value implicitly converts if it is in a happy condition. Should it be in an unhappy condition it might return a default/error value (for area 0 or -1 might seem reasonable). Alternately it could throw an exception on this side of the interface/language boundary.
... foo() {
var area_calc = new AreaCalculator();
return area_calc.CalculateArea(x, y) * z;
}
var volume = foo();
if (volume <= 0)
{
//handle error
}
vs.
... foo() {
var area_calc = new AreaCalculator();
return area_calc.CalculateArea(x, y) * z;
}
try { var volume = foo(); }
catch(...)
{
//handle error
}
By passing out a bad, or possibly bad value, it places a lot of onus on the user to validate the data. This is a source of bugs, because as far as the compiler is concerned the return value is a legitimate integer. If something was not checked you'll discover it when things go wrong. The second case mixes the best of both worlds by allowing exceptions to handle unhappy paths, while happy paths follow normal processing. Unfortunately it does force the user to handle exceptions wisely, which is hard.
Just to be clear an Unhappy path is a case unknown to business logic (the domain of exception), failing to validate is a happy path because you know how to handle that by the business rules (the domain of rules).
The ultimate solution would be one which allows all scenarios (within reason).
- The user should be able to query for a bad condition, and handle it immediately
- The user should be able to operate on the enriched type as if the happy path had been followed and propagate the error details.
- The user should be able to extract the happy path value through casting (implicit/explicit as is reasonable), generating an exception for unhappy paths.
- The user should be able to extract the happy path value, or use a default (supplied or not)
Something like:
Rich::value_type value_or_default(Rich&, Rich::value_type default_value = ...);
bool bad(Rich&);
...unhappy path report... bad_state(Rich&);
Rich& assert_not_bad(Rich&);
class Rich
{
public:
typedef ... value_type;
operator value_type() { assert_not_bad(*this); return ...value...; }
operator X(...) { if (bad(*this)) return ...propagate badness to new value...; /*operate and generate new value*/; }
}
//check
if (bad(x))
{
var report = bad_state(x);
//handle error
}
//rethrow
assert_not_bad(x);
var result = (assert_not_bad(x) + 23) / 45;
//propogate
var y = x * 23;
//implicit throw
Rich::value_type val = x;
var val = ((Rich::value_type)x) + 34;
var val2 = static_cast<Rich::value_type>(x) % 3;
//default value
var defaulted = value_or_default(x);
var defaulted_to = value_or_default(x, 55);
add a comment |
Sometimes throwing an exception is not the best method. Not least due to stack unwinding, but sometimes because catching an exception is problematic, particularly along language or interface seams.
The best way to handle this is to return an enriched data-type. This data type has enough state to describe all of the happy paths, and all of the unhappy paths. The point is, if you interact with this function (member/global/otherwise) you will be forced to handle the outcome.
That being said this enriched data-type should not force action. Imagine in your area example something like var area_calc = new AreaCalculator(); var volume = area_calc.CalculateArea(x, y) * z;
. Seems useful volume
should contain the area multiplied by depth - that could be a cube, cylinder, etc...
But what if the area_calc service was down? Then area_calc .CalculateArea(x, y)
returned a rich datatype containing an error. Is it legal to multiply that by z
? Its a good question. You could force users to handle the checking immediately. This does however break up the logic with error handling.
var area_calc = new AreaCalculator();
var area_result = area_calc.CalculateArea(x, y);
if (area_result.bad())
{
//handle unhappy path
}
var volume = area_result.value() * z;
vs
var area_calc = new AreaCalculator();
var volume = area_calc.CalculateArea(x, y) * z;
if (volume.bad())
{
//handle unhappy path
}
The essentially logic is spread over two lines and divided by error handling in the first case, while the second case has all the relevant logic on one line followed by error handling.
In that second case volume
is a rich data type. Its not just a number. This makes storage larger, and volume
will still need to be investigated for an error condition. Additionally volume
might feed other calculations before the user chooses to handle the error, allowing it to manifest in several disparate locations. This might be good, or bad depending on the specifics of the situation.
Alternately volume
could be just a plain data type - just a number, but then what happens to the error condition? It could be that the value implicitly converts if it is in a happy condition. Should it be in an unhappy condition it might return a default/error value (for area 0 or -1 might seem reasonable). Alternately it could throw an exception on this side of the interface/language boundary.
... foo() {
var area_calc = new AreaCalculator();
return area_calc.CalculateArea(x, y) * z;
}
var volume = foo();
if (volume <= 0)
{
//handle error
}
vs.
... foo() {
var area_calc = new AreaCalculator();
return area_calc.CalculateArea(x, y) * z;
}
try { var volume = foo(); }
catch(...)
{
//handle error
}
By passing out a bad, or possibly bad value, it places a lot of onus on the user to validate the data. This is a source of bugs, because as far as the compiler is concerned the return value is a legitimate integer. If something was not checked you'll discover it when things go wrong. The second case mixes the best of both worlds by allowing exceptions to handle unhappy paths, while happy paths follow normal processing. Unfortunately it does force the user to handle exceptions wisely, which is hard.
Just to be clear an Unhappy path is a case unknown to business logic (the domain of exception), failing to validate is a happy path because you know how to handle that by the business rules (the domain of rules).
The ultimate solution would be one which allows all scenarios (within reason).
- The user should be able to query for a bad condition, and handle it immediately
- The user should be able to operate on the enriched type as if the happy path had been followed and propagate the error details.
- The user should be able to extract the happy path value through casting (implicit/explicit as is reasonable), generating an exception for unhappy paths.
- The user should be able to extract the happy path value, or use a default (supplied or not)
Something like:
Rich::value_type value_or_default(Rich&, Rich::value_type default_value = ...);
bool bad(Rich&);
...unhappy path report... bad_state(Rich&);
Rich& assert_not_bad(Rich&);
class Rich
{
public:
typedef ... value_type;
operator value_type() { assert_not_bad(*this); return ...value...; }
operator X(...) { if (bad(*this)) return ...propagate badness to new value...; /*operate and generate new value*/; }
}
//check
if (bad(x))
{
var report = bad_state(x);
//handle error
}
//rethrow
assert_not_bad(x);
var result = (assert_not_bad(x) + 23) / 45;
//propogate
var y = x * 23;
//implicit throw
Rich::value_type val = x;
var val = ((Rich::value_type)x) + 34;
var val2 = static_cast<Rich::value_type>(x) % 3;
//default value
var defaulted = value_or_default(x);
var defaulted_to = value_or_default(x, 55);
Sometimes throwing an exception is not the best method. Not least due to stack unwinding, but sometimes because catching an exception is problematic, particularly along language or interface seams.
The best way to handle this is to return an enriched data-type. This data type has enough state to describe all of the happy paths, and all of the unhappy paths. The point is, if you interact with this function (member/global/otherwise) you will be forced to handle the outcome.
That being said this enriched data-type should not force action. Imagine in your area example something like var area_calc = new AreaCalculator(); var volume = area_calc.CalculateArea(x, y) * z;
. Seems useful volume
should contain the area multiplied by depth - that could be a cube, cylinder, etc...
But what if the area_calc service was down? Then area_calc .CalculateArea(x, y)
returned a rich datatype containing an error. Is it legal to multiply that by z
? Its a good question. You could force users to handle the checking immediately. This does however break up the logic with error handling.
var area_calc = new AreaCalculator();
var area_result = area_calc.CalculateArea(x, y);
if (area_result.bad())
{
//handle unhappy path
}
var volume = area_result.value() * z;
vs
var area_calc = new AreaCalculator();
var volume = area_calc.CalculateArea(x, y) * z;
if (volume.bad())
{
//handle unhappy path
}
The essentially logic is spread over two lines and divided by error handling in the first case, while the second case has all the relevant logic on one line followed by error handling.
In that second case volume
is a rich data type. Its not just a number. This makes storage larger, and volume
will still need to be investigated for an error condition. Additionally volume
might feed other calculations before the user chooses to handle the error, allowing it to manifest in several disparate locations. This might be good, or bad depending on the specifics of the situation.
Alternately volume
could be just a plain data type - just a number, but then what happens to the error condition? It could be that the value implicitly converts if it is in a happy condition. Should it be in an unhappy condition it might return a default/error value (for area 0 or -1 might seem reasonable). Alternately it could throw an exception on this side of the interface/language boundary.
... foo() {
var area_calc = new AreaCalculator();
return area_calc.CalculateArea(x, y) * z;
}
var volume = foo();
if (volume <= 0)
{
//handle error
}
vs.
... foo() {
var area_calc = new AreaCalculator();
return area_calc.CalculateArea(x, y) * z;
}
try { var volume = foo(); }
catch(...)
{
//handle error
}
By passing out a bad, or possibly bad value, it places a lot of onus on the user to validate the data. This is a source of bugs, because as far as the compiler is concerned the return value is a legitimate integer. If something was not checked you'll discover it when things go wrong. The second case mixes the best of both worlds by allowing exceptions to handle unhappy paths, while happy paths follow normal processing. Unfortunately it does force the user to handle exceptions wisely, which is hard.
Just to be clear an Unhappy path is a case unknown to business logic (the domain of exception), failing to validate is a happy path because you know how to handle that by the business rules (the domain of rules).
The ultimate solution would be one which allows all scenarios (within reason).
- The user should be able to query for a bad condition, and handle it immediately
- The user should be able to operate on the enriched type as if the happy path had been followed and propagate the error details.
- The user should be able to extract the happy path value through casting (implicit/explicit as is reasonable), generating an exception for unhappy paths.
- The user should be able to extract the happy path value, or use a default (supplied or not)
Something like:
Rich::value_type value_or_default(Rich&, Rich::value_type default_value = ...);
bool bad(Rich&);
...unhappy path report... bad_state(Rich&);
Rich& assert_not_bad(Rich&);
class Rich
{
public:
typedef ... value_type;
operator value_type() { assert_not_bad(*this); return ...value...; }
operator X(...) { if (bad(*this)) return ...propagate badness to new value...; /*operate and generate new value*/; }
}
//check
if (bad(x))
{
var report = bad_state(x);
//handle error
}
//rethrow
assert_not_bad(x);
var result = (assert_not_bad(x) + 23) / 45;
//propogate
var y = x * 23;
//implicit throw
Rich::value_type val = x;
var val = ((Rich::value_type)x) + 34;
var val2 = static_cast<Rich::value_type>(x) % 3;
//default value
var defaulted = value_or_default(x);
var defaulted_to = value_or_default(x, 55);
answered 3 hours ago
Kain0_0Kain0_0
2,915315
2,915315
add a comment |
add a comment |
Thanks for contributing an answer to Software Engineering Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e) {
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom)) {
StackExchange.using('gps', function() { StackExchange.gps.track('embedded_signup_form.view', { location: 'question_page' }); });
$window.unbind('scroll', onScroll);
}
};
$window.on('scroll', onScroll);
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fsoftwareengineering.stackexchange.com%2fquestions%2f386702%2fhaving-a-flag-to-indicate-if-we-should-throw-errors%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e) {
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom)) {
StackExchange.using('gps', function() { StackExchange.gps.track('embedded_signup_form.view', { location: 'question_page' }); });
$window.unbind('scroll', onScroll);
}
};
$window.on('scroll', onScroll);
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e) {
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom)) {
StackExchange.using('gps', function() { StackExchange.gps.track('embedded_signup_form.view', { location: 'question_page' }); });
$window.unbind('scroll', onScroll);
}
};
$window.on('scroll', onScroll);
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
var $window = $(window),
onScroll = function(e) {
var $elem = $('.new-login-left'),
docViewTop = $window.scrollTop(),
docViewBottom = docViewTop + $window.height(),
elemTop = $elem.offset().top,
elemBottom = elemTop + $elem.height();
if ((docViewTop elemBottom)) {
StackExchange.using('gps', function() { StackExchange.gps.track('embedded_signup_form.view', { location: 'question_page' }); });
$window.unbind('scroll', onScroll);
}
};
$window.on('scroll', onScroll);
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
1
Is the flag ever set to true in the code base?
– mmathis
8 hours ago
I changed your line "I agree they should be ignored," to "I agree they shouldn't be ignored,", as I think that's what you meant. Please revert or edit further if that was not your intent.
– mmathis
7 hours ago
@mmathis Thanks for the corrections. To answer your previous question, this is for a new class in our code, thus there aren't any calls to this part of code yet.
– Nicolas
7 hours ago
2
c# has a convention for this Try-Parse patter. more info: docs.microsoft.com/en-us/dotnet/standard/design-guidelines/… The flag is not match with this pattern.
– Peter
5 hours ago