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

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.
@JohnSaunders: No, you don't, the lambda closes over it: ideone.com/pjxEio
@T.J.Crowder thank you so much but what about different response types ?
@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

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.

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: 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

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

@SergeyBerezovskiy: why would that be a maintenance problem?
@JohnSaunders reading and understanding is a part of maintenance
@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.
(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

Linked

Hot Network Questions