2
\$\begingroup\$

I've a web service class to handle all requests to the external service.

All the methods are very similar, so I want to refactor this into DRY code.

This is an app in Xamarin.Forms. I'm making the external api so if anything should be changed there I can.

class WebService : IWebService
{
    public WebService(IRestClient restClient, IConnectivity connectivity)
    {
        RestClient = restClient;
        Connectivity = connectivity;
    }

    IRestClient RestClient { get; }
    IConnectivity Connectivity { get; }
    string SessionId { get; set; }
    public bool HasSession => !string.IsNullOrEmpty(SessionId);

    public ServiceResponse Login(string username, string password)
    {
        if (!Connectivity.IsConnected)
        {
            return new ServiceResponse
            {
                ErrorMessage = "No internet connection.",
                Success = false
            };
        }

        var request = new RestRequest("security/login", Method.POST);
        request.AddParameter("Username", username);
        request.AddParameter("Password", password);

        var response = RestClient.Execute<LoginResponse>(request);
        if (response.IsSuccessful)
        {
            SessionId = response.Data.SessionId;
            return new ServiceResponse
            {
                Success = true
            };
        }
        else
        {
            var responseMessage = JsonUtils.DeserializeAnonymousType(response.Content, new { Message = "" });
            if (!string.IsNullOrEmpty(responseMessage.Message))
                return new ServiceResponse
                {
                    ErrorMessage = responseMessage.Message,
                    Success = false
                };
            else
                return new ServiceResponse
                {
                    ErrorMessage = response.ErrorMessage,
                    Success = false
                };
        }
    }

    public ServiceResponse<IEnumerable<Scanner>> GetScanners()
    {
        if (!Connectivity.IsConnected)
        {
            return new ServiceResponse<IEnumerable<Scanner>>
            {
                ErrorMessage = "No internet connection.",
                Success = false
            };
        }

        var request = new RestRequest("scan/getscanners", Method.GET);
        request.AddHeader("SessionId", SessionId);

        var response = RestClient.Execute<GetScannersResponse>(request);
        if (response.IsSuccessful)
        {
            return new ServiceResponse<IEnumerable<Scanner>>
            {
                Data = response.Data.Scanners,
                Success = true
            };
        }
        else
        {
            var responseMessage = JsonUtils.DeserializeAnonymousType(response.Content, new { Message = "" });
            if (!string.IsNullOrEmpty(responseMessage.Message))
                return new ServiceResponse<IEnumerable<Scanner>>
                {
                    ErrorMessage = responseMessage.Message,
                    Success = false
                };
            else
                return new ServiceResponse<IEnumerable<Scanner>>
                {
                    ErrorMessage = response.ErrorMessage,
                    Success = false
                };
        }
    }

    public ServiceResponse<string> OpenScanningSession(ScannerType scannerType, Scanner scanner)
    {
        if (!Connectivity.IsConnected)
        {
            return new ServiceResponse<string>
            {
                ErrorMessage = "No internet connection.",
                Success = false
            };
        }

        var request = new RestRequest("scan/openscanningsession", Method.POST);
        request.AddHeader("SessionId", SessionId);
        request.AddParameter("ScannerType", scannerType.ToString());
        request.AddParameter("ScannerId", scanner.Id);

        var response = RestClient.Execute<OpenScanningSessionResponse>(request);
        if (response.IsSuccessful)
        {
            return new ServiceResponse<string>
            {
                Data = response.Data.ScanningSession,
                Success = true
            };
        }
        else
        {
            var responseMessage = JsonUtils.DeserializeAnonymousType(response.Content, new { Message = "" });
            if (!string.IsNullOrEmpty(responseMessage.Message))
                return new ServiceResponse<string>
                {
                    ErrorMessage = responseMessage.Message,
                    Success = false
                };
            else
                return new ServiceResponse<string>
                {
                    ErrorMessage = response.ErrorMessage,
                    Success = false
                };
        }
    }

    public ServiceResponse<ScanBarcodeResult> ScanBarcode(string barcode, string scanningSession, ScanAction scanAction)
    {
        if (!Connectivity.IsConnected)
        {
            return new ServiceResponse<ScanBarcodeResult>
            {
                ErrorMessage = "No internet connection.",
                Success = false
            };
        }

        var request = new RestRequest("scan/scanbarcode", Method.POST);
        request.AddParameter("Barcode", barcode);
        request.AddParameter("ScanningSession", scanningSession);
        request.AddParameter("ScanAction", scanAction.ToString());

        var response = RestClient.Execute<ScanBarcodeResult>(request);
        if (response.IsSuccessful)
            return new ServiceResponse<ScanBarcodeResult>
            {
                Data = response.Data,
                Success = true
            };
        else
            return new ServiceResponse<ScanBarcodeResult>
            {
                ErrorMessage = response.ErrorMessage,
                Success = false
            };
    }
}

ServiceResponse classes:

class ServiceResponse
{
    public bool Success { get; set; }
    public string ErrorMessage { get; set; }
}

class ServiceResponse<T>
{
    public bool Success { get; set; }
    public string ErrorMessage { get; set; }
    public T Data { get; set; }
}
\$\endgroup\$
  • \$\begingroup\$ All of the methods checking to see if they are connected seems ok to me, I would simply have some sort of local ServiceResponse field/constant which is returned if !Connectivitiy.IsConnected is false. Similarly, it might be worth creating a small helper method which takes in all of the required properties for creating a new RestRequest as well as an unknown number of header arguments (params might help with this). \$\endgroup\$ – Danny Goodall May 30 '18 at 10:43
1
\$\begingroup\$

Ternary Operator

The operator ?: could become very helpful to you as it allows you to inline an if-statement. Microsoft Documentation. For example,

if (response.IsSuccessful)
    return new ServiceResponse<ScanBarcodeResult>
    {
        Data = response.Data,
        Success = true
    };
else
    return new ServiceResponse<ScanBarcodeResult>
    {
        ErrorMessage = response.ErrorMessage,
        Success = false
    };

Can become,

return new ServiceResponse<ScanBarcodeResult>
{
    Data = response.IsSuccessful ? response.Data : null, // null or w/e the default value is for its datatype.
    ErrorMessage = response.IsSuccessful == false ? response.ErrorMessage : null, // null or w/e the default value is for its datatype.
    Success = response.IsSuccessful
};

CreateResponse Function

You could create a private function for creating a response since it seems to be a pretty common and uniform action in your code. Something like this perhaps,

private ServiceResponse<T> CreateResponse<T>(T data, string errorMessage, bool wasSuccessful)
{
    return new ServiceResponse<T>()
    {
        Data = wasSuccessful ? data : default(T),
        ErrorMessage = wasSuccessful == false ? errorMessage : null,
        Success = wasSuccessful
    };
}

CreateResponse<string>(response.Data, response.ErrorMessage, response.IsSuccessful);

All this does is make it to where you have to type less, there will be some cases where you won't want to do this. I feel like the best way to know is experience :)

Doing something like this might make your code a bit more concise (and therefore readable) since you don't have to create a new ServiceResponse multiple times a function as you are doing now, which is multiple lines of code.

Hope these tips help or give you some of your own ideas!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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