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
I advise against nesting try-catches.
Use two separate try-catches for each field (
model.PhoneNumber
andmodel.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.If all you need is one valid phone number, create a little helper method in your model called
EnsureValidState
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:
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.
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).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, theTryParse
should not wrap calls toParse
intry/catch
blocks, because exception handling is expensive. Instead it should use the same logic as theParse
method, but returnfalse
instead of throwing an exception.