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](https://msdn.microsoft.com/en-us/library/aa355056.aspx) — Tim, May 12 '15 at 01:03

score 6 · Accepted Answer · edited May 23 '17 at 11:58

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"));
    }
}

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`. — John Saunders, May 11 '15 at 22:26
@JohnSaunders: No, you don't, the lambda closes over it: http://ideone.com/pjxEio — T.J. Crowder, May 12 '15 at 06:20
@T.J.Crowder thank you so much but what about different response types ? — moath naji, Dec 18 '19 at 18:49
@moathnaji - If you need to support them, you can make `Worker` generic: `private T Worker(Func T> f) { /*...*/ }` (I think, that's off the top of my head and I haven't done C# generics in a few months.) — T.J. Crowder, Dec 18 '19 at 19:10

John Saunders · Answer 2 · 2015-05-11T22:11:23.800

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 '15 at 22:11

answered May 11 '15 at 22:05

John Saunders

160,644
26
247
397

I don't see how that helps - in fact if anything it adds a method? Note that each call is to a different endpoint – JamesSugrue May 11 '15 at 22:06
@JamesSugrue: it centralizes the creation of the proxy instance. However it needs to be created, it will be created in one place only. – John Saunders May 11 '15 at 22:15

Pajdziu · Answer 3 · 2015-05-11T22:32:30.797

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 '15 at 22:32

answered May 11 '15 at 22:10

Pajdziu

911
1
9
22

Maintenance hell detected – Sergey Berezovskiy May 11 '15 at 22:11
@SergeyBerezovskiy: why would that be a maintenance problem? – John Saunders May 11 '15 at 22:12
@JohnSaunders reading and understanding is a part of maintenance – Sergey Berezovskiy May 11 '15 at 22:13
@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. – John Saunders May 11 '15 at 22:14
1

`(authInfo, request) => soapClient.GetCarriers(authInfo, request)` can usually be replaced by `soapClient.GetCarriers` – Tim S. May 11 '15 at 22:16
@JohnSaunders I believe more than zero people is enough. I don't think this pattern is easier to follow than original OP using statements. IMO calling methods with two parameters one of which is lambda with two more parameters does not simplify code. If you'll run something like CodeRush maintenance cost check you will be surprised – Sergey Berezovskiy May 11 '15 at 22:21
@TimS.: that last edit won't build. `soapClient` is not in scope. – John Saunders May 11 '15 at 22:23
@JohnSaunders, Yeah, my bad. So T.J. Crowder's answer is correct. – Pajdziu May 11 '15 at 22:24
@KamilPajdzik: his answer is close. See my last comment to his answer. Wish I had time now, but have to run. – John Saunders May 11 '15 at 22:26

score 0 · Answer 4 · answered May 11 '15 at 22:12

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;
            }
        }
    }
}

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

4 Answers4