Try-Catch vs If-Else | Should I fight to use If-Else in this or just go with the Try-Catch?

Question

Summary

I've been given a task to set up a management software (For a small scale artist, so their hardware can definitely cope), however, I would prefer to make this as efficient as possible before giving it to them. The main functionality is done, and now it is mainly just touching up and optimizing.

Code

        DateTime DueDate;
        try
        {
            DateTime.TryParse(dteCommission.SelectedDate.Value.Date.ToShortDateString(),
            out DueDate);
        }
        catch(Exception E)
        {
            MessageBox.Show("Due Date wasn't set. Defaulting to current date.", "Alert",
                MessageBoxButton.OK, MessageBoxImage.Warning);
            DueDate = DateTime.Parse(DateTime.Now.ToShortDateString());
        }

Note: Exception e was only used to get it done quickly and the true exception is known. The error given is "Nullable object must have a value." System.InvalidOperationException

Question

Is it best to handle this as I am doing or would If-Else work better? And if so, how would I go about implementing it?

You should avoid exception handling in all cases unless there is an exceptional situation. That's why they are called exceptions. — Enigmativity, Mar 13 '19 at 04:28
There's multiple things going on here, and it's not clear whether they're relevant to your question. For example, you appear to be converting DateTimes to strings and back to DateTimes, and it's not clear to me why. If you want to remove the time of day component, just use the `DateTime.Date` property. But that's not the source of your exception, it just seems to be extra complexity unrelated to your question. — Weeble, Mar 13 '19 at 04:29
Whatever you do, try to always avoid catching any exception of type `Exception` unless you plan to rethrow it. — 41686d6564 stands w. Palestine, Mar 13 '19 at 04:29

Yogi · Answer 1 · 2019-03-13T04:24:44.207

3

As you are already using TryParse there is no need to use try ...catch block. Not just it's inefficient, it is also not clean. Just take the return value of DateTime.TryParse and take the decision.

var isDate = DateTime.TryParse(dteCommission.SelectedDate.Value.Date.ToShortDateString(),

and then, if (isDate){...} else {...}

edited Mar 13 '19 at 04:24

answered Mar 13 '19 at 04:23

Yogi

9,174
2
46
61

I would accept this, but unfortunately using `TryParse` threw an error. – John Smith Mar 13 '19 at 04:24
1

In that case, it is very likely that in '`dteCommission.SelectedDate.Value.Date' either the `Value` or `Date` is `null`, so you should check for the sanity of this object, and then use the above as suggested. – Yogi Mar 13 '19 at 04:26

ChizT · Accepted Answer · 2019-03-13T04:29:43.103

Exception e was only used to get it done quickly and the true exception is known. The error given is "Nullable object must have a value." System.InvalidOperationException

How would you know that in runtime that it would be a different exception? lets say NullReferenceException(for example) maybe. Remember that all exceptions implements Exception object.

Is it best to handle this as I am doing or would If-Else work better?

You need to handle errors better. You know that it could be Nullable so you need to check if it has value before proceeding. You should look out for warnings and handle them elegantly.

And if so, how would I go about implementing it?

try
{
    if(dteCommission.SelectedDate.HasValue) 
    { 
        DateTime.TryParse(dteCommission.SelectedDate.Value.Date.ToShortDateString(),
                    out DueDate); 
    } else{
        MessageBox.Show("Due Date wasn't set. Defaulting to current date.", "Alert",
                    MessageBoxButton.OK, MessageBoxImage.Warning);
                DueDate = DateTime.Parse(DateTime.Now.ToShortDateString());
    }
} 
catch(Exception e)
{
    Log.LogError(e);
    MessageBox.Show("Unhandle error occurred please call Admin", "Alert",
                    MessageBoxButton.OK, MessageBoxImage.Warning);
}

This was what I was after, thank you. Do you have any suggestions where I should go to revise error handling? — John Smith, Mar 13 '19 at 04:25

yekanchi · Answer 3 · 2019-03-13T04:36:54.467

If you are committed to use tryparse then it's a better approach to go with If-Else which depends on the output of the tryparse method. but if you are using Parse it's likely that you end up with one of these exceptions:

ArgumentNullException (if parameter value is null)
FormatException(If parameter value is other than integer value or not in proper format)
FormatException(if parameter value is out of integer range)

so it's better to go with exception handling.

for the first approach:

var isParsable = DateTime.TryParse(dteCommission.SelectedDate.Value.Date.ToShortDateString(),
out DueDate);
if (isParsable)
{
     //Continue With your Procedure
}
else
{
     MessageBox.Show("Due Date wasn't set. Defaulting to current date.", "Alert",
     MessageBoxButton.OK, MessageBoxImage.Warning);
}

for the second case you can go with:

DateTime DueDate;
try
{
     var DueDate = DateTime.TryParse(dteCommission.SelectedDate.Value.ToString());

}
catch (Exception E)
{
     MessageBox.Show("Due Date wasn't set. Defaulting to current date.", "Alert",
     MessageBoxButton.OK, MessageBoxImage.Warning);
     //also you can you the exception type to make it clear for use if it is
     // an exception of Null, Format or Argument
}

score 0 · Answer 4 · answered Mar 13 '19 at 04:26

I would like to suggest to use if else statement for such a scenario rather than the exception, it will be optimized as well and gives you an opportunity to give meaningfully message specific to the scenario.

Exception handling should be used only to handle unknown scenarios.

Try-Catch vs If-Else | Should I fight to use If-Else in this or just go with the Try-Catch?

4 Answers4