How to write a try catch statement where I need to check two things

Question

I have a method which have a try catch clause where I parse a mobile phone number. If parsing goes OK, the code outside the try continues to execute. If parsing encounters an error, I enter the catch and raise the error.

All this was OK until I got a request to check for another phone number.

I am not sure how to do it because later in the code I need at least one correct phone number (its not important which one).

If I put both parsing inside one try, I have a problem if first one is wrong and second is good because exception will be raised anyway.

try {
    model.mobilePhone = PhoneParser.Parse(m.mobile);
    model.alternativePhoneNumber = PhoneParser.Parse(m.alternativePhoneNumber);
}

catch (Exception) {
    _log.LogWarning("Error while parsing the phone number")
}

return model;

Maybe something like this? Try inside a catch?

try {
    model.mobilePhone = PhoneParser.Parse(m.mobile);
}

catch (Exception) {
    try {
         model.alternativePhoneNumber = PhoneParser.Parse(m.alternativePhoneNumber);
  }
     catch (Exception) {
         _log.LogWarning("Error while parsing the alternative phone number")
  }
    _log.LogWarning("Error while parsing the mobile phone number")
}

return model;

Do you have access to the `PhoneParser.Parse` method? If so, why not change it to be more like `int.TryParse`? — DavidG, Mar 20 '23 at 14:39
Why do these operations need to be combined at all? If the try/catch structure for parsing one value was working, why not just repeat that same structure for the second value? — David, Mar 20 '23 at 14:42
Two David's, nice :) for DavidG, I do have access, but inside it have several checks which are raising the exceptions, like if (phoneNumber.Length < 9) throw new ArgumentException("Invalid phone number"); if (phoneNumber == null) throw new ArgumentNullException(); if (phoneNumber.Length > 15) throw new OverflowException("Phone number with calling code cannot have more than 15 characters"); — sosNiLa, Mar 20 '23 at 14:48
@David, you mean try { model.mobilePhone = PhoneParser.Parse(m.mobile); } catch (Exception) { _log.LogWarning("Error while parsing the phone number") } try { model.alternativePhoneNumber = PhoneParser.Parse(m.alternativePhoneNumber); } catch (Exception) { _log.LogWarning("Error while parsing the phone number") } — sosNiLa, Mar 20 '23 at 14:49
@sosNiLa: Yes. Unless parsing the second value somehow *depends upon* the result of parsing the first value, I don't see why they need to be mashed together at all. — David, Mar 20 '23 at 14:51
@sosNiLa if you were to use the int.tryparse you will be able to null check the out value — Roe, Mar 20 '23 at 14:51
Since a phonenumber could start with a zero you could never int.parse it without losing a leading zero. Do you really wish to parse the string to int? Couldn't you implement regex to validate the phonenumber instead? — Roe, Mar 20 '23 at 14:56
The question is, what if the format of `m.mobile` is wrong. Is this field optional? Are you able to continue to the `alternativePhoneNumber`? Don't nest try/catch, use two different blocks. — Jeroen van Langen, Mar 20 '23 at 15:05
@Roe, it has to be a string because inside the same PhoneParser.Parse method, I am adding the country code. — sosNiLa, Mar 20 '23 at 15:09
@JeroenvanLangen, yes, I need at least one number to be correct. — sosNiLa, Mar 20 '23 at 15:09
I would create a separate function which does the `TryParse()` — Jeroen van Langen, Mar 20 '23 at 15:33
@Roe It should be self-evident that a phone number is not being stored as an `int`. Even if stored without separators (`.`, `-`, or `()`, you would only allow for ~25% of all possible phone numbers, and then only if you are using a 10-digit phone number like in the US without a country code at the beginning. If not constrained to that, then it's limited to significantly less than 25% of all possible numbers. — TylerH, Mar 21 '23 at 14:20
If you own the code of your phone number parser, **please don't throw** `OverflowException` in this case. This exception class has a [very specific meaning](https://learn.microsoft.com/en-us/dotnet/api/system.overflowexception?view=net-7.0) in c#, regarding arithmetic overflow in checked code blocks. — Rodrigo Rodrigues, Mar 21 '23 at 14:56

score 1 · Answer 1 · answered Mar 20 '23 at 14:43

I advise against nesting try-catches.

Use two separate try-catches for each field (model.PhoneNumber and model.AlternativePhoneNumber).

Although the best way of solving your issue would be creating a separate .TryParse() extension method for phone number and using that. This would also conform your code to DRY (do not repeat yourself) principle.

score 1 · Answer 2 · answered Mar 20 '23 at 15:19

If all you need is one valid phone number, create a little helper method in your model called EnsureValidState

public void EnsureValidState()
{
   if(this.mobilePhone == null && this.alternativePhoneNumber ==null) 
   {
     //no valid state, raise exception
   }
    
    //other validation for different rules.
}

Call this method, after all your properties are set and validate accordingly. You could even abstract this away into an abstract base class for your entities:

public abstract class BaseEntity
{
   private abstract void EnsureValidState();
}

If you want to go a step further have a look at the Hands on DDD example code over at https://github.com/PacktPublishing/Hands-On-Domain-Driven-Design-with-.NET-Core/blob/086cabf75211daa8df9d2117dec0181a0339749d/Marketplace.Framework/Entity.cs, that will give you an idea on how to implement a more generic way to ensure consistent updates on your entites and models.

score 1 · Accepted Answer · answered Mar 20 '23 at 16:02

The simplest answer is to have two separate try/catch blocks. Otherwise, the alternate phone number will only ever get added if the primary phone number fails (but it's likely that if they supplied both, both are valid).

try 
{
    model.mobilePhone = PhoneParser.Parse(m.mobile);
}
catch (Exception e)
{
    _log.LogWarning($"Error while parsing the mobile phone number:\r\n{e}")
}

try 
{
    model.alternativePhoneNumber = PhoneParser.Parse(m.alternativePhoneNumber);
}
catch (Exception e) 
{
    _log.LogWarning($"Error while parsing the alternative phone number:\r\n{e}")
}

return model;

As others have suggested, implementing a TryParse might be helpful, but then you'll lose the exception information that you're currently logging now. Also, the TryParse should not wrap calls to Parse in try/catch blocks, because exception handling is expensive. Instead it should use the same logic as the Parse method, but return false instead of throwing an exception.

How to write a try catch statement where I need to check two things

3 Answers3