How to refactor "using" statement to avoid code duplication?

Question

Let's suppose I have the following methods:

 public string GetSchedules(string request)
    {
        using (var soapClient = new ServiceReference1.CustomDataTimetableToolKitServicesSoapClient(EndpointConfiguratioName, Endpoint))
        {
            return soapClient.GetSchedules(AuthenticationInfo, request);
        }
    }

    public string GetCountryList(string request)
    {
        using (var soapClient = new ServiceReference1.CustomDataTimetableToolKitServicesSoapClient(EndpointConfiguratioName, Endpoint))
        {
            return soapClient.GetCountryList(AuthenticationInfo, request);
        }
    }

    public string GetCarriers(string request)
    {
        using (var soapClient = new ServiceReference1.CustomDataTimetableToolKitServicesSoapClient(EndpointConfiguratioName, Endpoint))
        {
            return soapClient.GetCarriers(AuthenticationInfo, request);
        }
    }

As you can see the only different thing is the name of the method invoked. How could I refactor these methods to apply "using" statement only once and avoid code duplication?

Best practices are to not use the using statement with WCF clients - MSDN — Tim
– Tim, Commented May 12, 2015 at 1:03

Community · Accepted Answer · 2017-05-23 11:58:11Z

6

To me, what you have is fine, but if you want to factor those out you can use Func and lambdas. Something along these lines:

public string GetSchedules(string request)
{
    return Worker((c) => c.GetSchedules(AuthenticationInfo, request));
}

public string GetCountryList(string request)
{
    return Worker((c) => c.GetCountryList(AuthenticationInfo, request));
}

public string GetCarriers(string request)
{
    return Worker((c) => c.GetCarriers(AuthenticationInfo, request));
}

private string Worker(Func<SoapClientClassGoesHere, string> f)
{
    using (var soapClient = new ServiceReference1.CustomDataTimetableToolKitServicesSoapClient(EndpointConfiguratioName, Endpoint))
    {
        return f(soapClient);
    }
}

Func<A, R> means "a function that takes an argument of type A and returns a value of type R" (and you can have Func<A, B, R> for a function that takes two arguments, etc.).

More about Func<> and lambdas in this question and this question (and many more, it's a rich topic).

Here's a live example on ideone.com (a very silly live example, but it demonstrates the concept):

using System;
using System.Collections.Generic;

class Foo {
    public string GetSchedules(string request)
    {
        return Worker((c) => c[request]);
    }

    public string GetCountryList(string request)
    {
        return Worker((c) => c[request].ToUpper());
    }

    public string GetCarriers(string request)
    {
        return Worker((c) => c[request].ToLower());
    }

    private string Worker(Func<Dictionary<string,string>, string> f)
    {
        var d = new Dictionary<string, string>();
        d.Add("1", "One");
        d.Add("2", "Two");
        d.Add("3", "Three");
        return f(d);
    }
}

public class Test
{
    public static void Main()
    {
        var f = new Foo();
        Console.WriteLine(f.GetSchedules("1"));
        Console.WriteLine(f.GetCountryList("1"));
        Console.WriteLine(f.GetCarriers("1"));
    }
}

edited May 23, 2017 at 11:58

CommunityBot

11 silver badge

answered May 11, 2015 at 22:10

T.J. Crowder

1.1m201 gold badges2k silver badges2k bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

John Saunders Over a year ago

You need to pass request to the delegate, so you need to pass it to Worker, and you'll need to update the signature of the Func.

T.J. Crowder Over a year ago

@JohnSaunders: No, you don't, the lambda closes over it: ideone.com/pjxEio

moath naji Over a year ago

@T.J.Crowder thank you so much but what about different response types ?

T.J. Crowder Over a year ago

@moathnaji - If you need to support them, you can make Worker generic: private T Worker<T>(Func<Dictionary<string, string> T> f) { /*...*/ } (I think, that's off the top of my head and I haven't done C# generics in a few months.)

John Saunders · Accepted Answer · 2015-05-11 22:11:23Z

2

There's really not much duplication there. Still,

public ServiceReference1.CustomDataTimetableToolKitServicesSoapClient NewClient()
{
    return new ServiceReference1.CustomDataTimetableToolKitServicesSoapClient(EndpointConfiguratioName, Endpoint)
}

using (var client = NewClient()) {
    return soapClient.GetCountryList(AuthenticationInfo, request);
}

Also, since all of your methods take a string parameter and return a string, it would be easy to write a single method to call them all, passing the operation to call as a delegate. Unfortunately, I don't have the time to write that for you right now.

edited May 11, 2015 at 22:11

answered May 11, 2015 at 22:05

John Saunders

162k26 gold badges252 silver badges403 bronze badges

2 Comments

JamesSugrue Over a year ago

I don't see how that helps - in fact if anything it adds a method? Note that each call is to a different endpoint

John Saunders Over a year ago

@JamesSugrue: it centralizes the creation of the proxy instance. However it needs to be created, it will be created in one place only.

Pajdziu · Accepted Answer · 2015-05-11 22:32:30Z

2

You can use lambda function in such way:

public string GetCarriers(string request)
{
    return Get((authInfo, request) => soapClient.GetCarriers(authInfo, request), request);
}

...

public string Get(Func<AuthenticationInfo, string, string> action, string request) {
    using (var soapClient = new ServiceReference1.CustomDataTimetableToolKitServicesSoapClient(EndpointConfiguratioName, Endpoint))
    {
        return action(AuthenticationInfo, request)
    }
}

I don't know if that compiles but you get the idea.

Edit: As @Tim S. noticed, this code could probably be shorter:

public string GetCarriers(string request)
{
    return Get(soapClient.GetCarriers, request);
}

...

public string Get(Func<AuthenticationInfo, string, string> action, string request) {
    using (var soapClient = new ServiceReference1.CustomDataTimetableToolKitServicesSoapClient(EndpointConfiguratioName, Endpoint))
    {
        return action(AuthenticationInfo, request)
    }
}

Edit 2: The client was out of scope. So correct code is:

public string GetCarriers(string request)
{
    return Get((client, authInfo, request) => client.GetCarriers(authInfo, request));
}

...

public string Get(Func<ISoapClient, AuthenticationInfo, string, string> action, string request) {
    using (var soapClient = new ServiceReference1.CustomDataTimetableToolKitServicesSoapClient(EndpointConfiguratioName, Endpoint))
    {
        return action(soapClient, AuthenticationInfo, request)
    }
}

edited May 11, 2015 at 22:32

answered May 11, 2015 at 22:10

Pajdziu

9201 gold badge9 silver badges23 bronze badges

9 Comments

Sergey Berezovskiy Over a year ago

Maintenance hell detected

John Saunders Over a year ago

@SergeyBerezovskiy: why would that be a maintenance problem?

Sergey Berezovskiy Over a year ago

@JohnSaunders reading and understanding is a part of maintenance

John Saunders Over a year ago

@SergeyBerezovskiy: but how many people will need to read (and understand) that code? Besides, the pattern will be easy to follow if another method needs to be added.

Tim S. Over a year ago

(authInfo, request) => soapClient.GetCarriers(authInfo, request) can usually be replaced by soapClient.GetCarriers

|

Breeze · Accepted Answer · 2015-05-11 22:12:15Z

If your Projekt isn't very big you could use the following (it's a bit messy and makes it easy to make a mistake):

public string getX (string request, string x)
{
    using (var soapClient = new ServiceReference1.CustomDataTimetableToolKitServicesSoapClient(EndpointConfiguratioName, Endpoint))
    {
        switch (x)
        {
            case "schedules":
                return soapClient.GetSchedules(AuthenticationInfo, request);
                break;
            case "countryList":
                return soapClient.GetCountryList(AuthenticationInfo, request);
                break;
            case "carriers":
                return soapClient.GetCarriers(AuthenticationInfo, request);
                break;
            }
        }
    }
}

Collectives™ on Stack Overflow

How to refactor "using" statement to avoid code duplication?

4 Answers 4

4 Comments

2 Comments

9 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

4 Comments

2 Comments

9 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related