HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Does using a lambda make this code more DRY?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thismakemoreusingdoesdrycodelambda

Problem

I started out with a function having code duplication like this:

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