patterncsharpMinor
Does using a lambda make this code more DRY?
Viewed 0 times
thismakemoreusingdoesdrycodelambda
Problem
I started out with a function having code duplication like this:
Then, because I had copy/pasted to make that code, I started trying to eliminate that duplication.
First I extracted a method:
However, while that eliminated the duplication it also created a duplication of
However, I would have been confused by that code before I learned about lamdas so I wonder, was the last step a step in the wrong direction?
private static void GetAndSaveResources(string url)
{
...
if (url.Contains("[" + "access_id" + "]"))
{
RestCall.UriVariables.Add("access_id", 1);
}
if (url.Contains("[" + "affiliate_status" + "]"))
{
RestCall.UriVariables.Add("affiliate_status", "approved");
}
...
}Then, because I had copy/pasted to make that code, I started trying to eliminate that duplication.
First I extracted a method:
private static void ConfigureUriVariable(string url, string variable, object value)
{
if (url.Contains("[" + variable + "]"))
{
RestCall.UriVariables.Add(variable, value);
}
}However, while that eliminated the duplication it also created a duplication of
string url accross two functions so I futher reduced it to:Action setUriVariable = (variable, value) =>
{
if (url.Contains("[" + variable + "]"))
{
RestCall.UriVariables.Add(variable, value);
}
};
setUriVariable("access_id", 1);
setUriVariable("affiliate_status", "approved");However, I would have been confused by that code before I learned about lamdas so I wonder, was the last step a step in the wrong direction?
Solution
First, yes, I do think your solution with a lambda is a good choice. But it's worth to also consider other options:
-
Translate the closure into an object. That would mean creating a class, with a constructor that takes
-
Go the other way around: create a
-
Translate the closure into an object. That would mean creating a class, with a constructor that takes
url (and possibly RestCall, if it's an instance property) and stores it into a filed. It also has a method that actually does the work. Basically, this is the same as you lambda, only much more code. I think this is probably not a good solution in this case.-
Go the other way around: create a
Dictionary (or Tuple[] if order is important for you; or an array of anonymous type) with your parameters and then iterate over that:var items = new Dictionary
{
{ "access_id", 1 },
{ "affiliate_status", "approved" }
};
foreach (var item in items)
{
if (url.Contains("[" + item.Key + "]"))
{
RestCall.UriVariables.Add(item.Key, item Value);
}
};Code Snippets
var items = new Dictionary<string, object>
{
{ "access_id", 1 },
{ "affiliate_status", "approved" }
};
foreach (var item in items)
{
if (url.Contains("[" + item.Key + "]"))
{
RestCall.UriVariables.Add(item.Key, item Value);
}
};Context
StackExchange Code Review Q#10255, answer score: 4
Revisions (0)
No revisions yet.