skip to Main Content

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;

3

Answers


  1. 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.

    Login or Signup to reply.
  2. 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.

    Login or Signup to reply.
  3. 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:rn{e}")
    }
    
    try 
    {
        model.alternativePhoneNumber = PhoneParser.Parse(m.alternativePhoneNumber);
    }
    catch (Exception e) 
    {
        _log.LogWarning($"Error while parsing the alternative phone number:rn{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.

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search