CODEWITHSUNDEEP
c#
PositiveGuy
PositiveGuy

Reputation: 47763

Handling return value from Web Service Call Wrapper

I created this method below which makes an HTTP call to a 3rd party API. I just want opinions on if I'm handling this the best way. If the call fails, I need to return the ExistsInList bool value only if the response is not null. But in the last return statement, wouldn't I have to essentially do another return selectResponse == null ? false : selectResponse.ExistsInList; to check for null first just like the previous return in the catch?

Just seems redundant the way I'm approaching this and I don't know if I really need to check for null again in the final return but I figure yes, because you can't always rely on the response to give you a valid response even if there were no errors picked up.

public static bool UserExistsInList(string email, string listID)
{
    SelectRecipientRequest selectRequest = new SelectRecipientRequest(email, listID);
    SelectRecipientResponse selectResponse = null;

    try
    {
        selectResponse = (SelectRecipientResponse)selectRequest.SendRequest();
    }
    catch (Exception)
    {
        return selectResponse == null ? false : selectResponse.ExistsInList;
    }

    return selectResponse.ExistsInList;
}

Upvotes: 0

Views: 425

Answers (1)

John Saunders
John Saunders

Reputation: 161821

Suggestion #1: don't eat exceptions! You have no idea whatsoever what kind of exceptions you're ignoring. You're assuming that any exceptions at that point imply a problem with the third-party API, but that may not be the case.

My suggestion is to at least log the exception and then ignore it. Be certain to look at the log during development so you can see what kind of exceptions you're getting. Look at the code in the wrapper to understand which exceptions you might reasonably receive (or rather, which ones mean that something reasonable has happened, like a communication failure).

Then, change this code to catch only the exceptions that imply a problem with that API. The rest should be permitted to propagate, to be handled by code that knows how to handle them, or to be handled by code that will log them properly.

See "Asked to fix bugs in a program, you find over 100 instances of…" for an example of someone who had to clean up from code like this.

Upvotes: 4

Related Questions