Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

We've started working on a mobile app using Xamarin. There's a core library that is to be shared among Android and iOS apps. In the core library there will be several functions for communicating with a web service. I've had a discussion with my brother on what the best approach would be, we both have a different opinion, so we'd like to hear your views on the matter. And perhaps there's an even better approach we both didn't think of yet.

The web service we talk to has some predefined HTTP status codes for certain errors. For example when performing verification of an account and information is incorrect, the web service could return a status code 400 and some other errors might result in a different status code.

My approach was the following:

public async Task<string> VerifyAccountAsync (string email, string password) {
        // prevent null values
        email = email ?? "";
        password = password ?? "";

        var queryString = String.Format ("/VerifyAccount?email={0}&password={1}", email, password);
        var request = WebRequest.Create (_accountUrlString + queryString);
        string result = null;

        try {
                using (var response = await request.GetResponseAsync ()) {
                        using (var stream = response.GetResponseStream ()) {
                                using (var reader = new StreamReader (stream)) {
                                        result = reader.ReadToEnd ();
                                }
                        }
                }
        } catch (WebException ex) {
                // a protocol exception means we got a response, but the status code implies some error occured.
                if (ex.Status == WebExceptionStatus.ProtocolError) {
                        var httpResponse = (HttpWebResponse)ex.Response;

                        switch (httpResponse.StatusCode) {
                        case HttpStatusCode.BadRequest: // 400
                                throw new DepotServiceException (httpResponse, "User could not be found.");
                        case HttpStatusCode.InternalServerError: // 500
                                throw new DepotServiceException (httpResponse, "Some error");
                        default: // TODO: use the response stream to set the error message.
                                throw new DepotServiceException (httpResponse, "Undefined error ...");
                        }                                                                
                } else {
                        // some other exception occured, we'll just rethrow it (should not be ignored)
                        throw ex;
                }
        }

        return result;
}

My reasoning was the following:

  • Since the web service uses some predefined status codes for certain errors, we should use a typed exception (DepotServiceException) to make it more clear for the consumer of the core library what's the origination of the error.
  • Handling exceptions this way in the core library, will prevent having to use if/else constructions in the iPhone / Android visual layers. In the visual layer we won't have to check what the status code was, to show the appropriate error message.

My brother's approach is the following:

public async Task<DepotServiceResult> RetrieveUrl(string url) {
        DepotServiceResult serviceResult = new DepotServiceResult();
        WebRequest request = WebRequest.Create(url);

        try {
                using (var response = await request.GetResponseAsync()) {
                        var webresponse = (HttpWebResponse)response;
                        using (var stream = response.GetResponseStream()) {
                                serviceResult.statuscode = webresponse.StatusCode;
                                using (var streamReader = new StreamReader(stream)) {
                                        serviceResult.response = streamReader.ReadToEnd();
                                }
                        }
                }
        } catch (WebException ex) {
                serviceResult.errorDescription = ex.ToString();
        }

        return serviceResult;
}


public async Task<DepotServiceResult> VerifyAccountAsync(string email, string password) {
        var url = _urlBase + "api/account/VerifyAccount?email={0}&password={1}";
        email = WebUtility.HtmlEncode (email);
        password = WebUtility.HtmlEncode (password);

        url = String.Format (url, email, password);
        return await RetrieveUrl(url);
}

public class DepotServiceResult {
     public string errorDescription;
     public HttpStatusCode statuscode;
     public string response;
}

My brother prefers his approach because he doesn't want the core layer of the app to handle the error. He prefers to handle the error in the UI layer of the app. So for his approach he choose to create a DepotServiceResult object that contains the whole result of the operation, including errors if any occurred.

Personally, I believe my approach is the best, though I have difficulty in explaining why it's the better approach. So if anyone would agree with me, could I get some solid reasons on why it's the better approach? Of course, my brother prefers his approach and if his approach is better, I certainly would like to hear good reasoning on why it's the better one.

If there's an even better approach compared to our preferences, we'd like to hear it as well.

share|improve this question
add comment

2 Answers

up vote 1 down vote accepted

Yours is better in the fact that you treat your corelib as an API. Throwing meaningful exceptions upwards to the application.

His is better for being precautious by encoding the email and password entries.

I really don´t see the need of the DepotServiceResult if not to avoid costly exception handling (which in this case should include a boolean property HasError).

share|improve this answer
    
What do you mean with your second point? More specifically, how would my brothers approach not cluster the UIThread (compared to my approach - we both use async / await) ? –  Wolfgang Schreurs Jan 15 at 23:21
1  
Sorry, that´s just a legibility issue of your code, the usage of async wasn´t prominent. I´ve edited my reply. I also agree with @LynHeadley in the exceptions should be "exceptional" and not common scenários. But from my perspective, what the developer treats as exceptional is of his own choosing. Imagine if loading of the page/form/view checks/depends on connectivity status before showing the eminent e-mail verification/retrieval page. Then failure of this process(retrieving response) could be considered exceptional. –  Ade Alejo Jan 16 at 22:21
add comment

The main difference seems to be that your code converts error responses into exceptions while your brother simply returns them. I believe that exceptions should be used to indicate programming bugs or emergency situations that threaten the integrity of a piece of the system. From this perspective, the library should probably not throw an exception because there is no bug in the code and no emergency as far as it knows. Rather, an external service is down or a user has entered bad input, both of which are basically "normal" situations.

I agree with your brother.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.