How to avoid repetitive statements in this code?

Question

I have the following code:

public static void MyFunc(string Title,string Artist,string Music)

        {
            List<string> g;
  
            if  ( !string.IsNullOrEmpty(Title))
            {
                g.Add( "Title " + " operator " + Title);

            }
            if  ( !string.IsNullOrEmpty(Artist))
            {
                g.Add("Artist " + " operator " + Artist);

            }
            if ( !string.IsNullOrEmpty(Music))
            {
                g.Add("Music " + " operator " + Music);

            }

        }

I want to avoid repetitive statements (in fact I have more variables (Year, Language,etc) in my real code).

How can I rewrite this code by calling 3 times the same function ?

Is it possible ? Thanks

Simplify it how? You want fewer lines? No if statements? Faster performance (if so, what's your hardware, what compiler are you using, etc)? — TylerH, Aug 12 '21 at 14:08
@TylerH: Usually, I'm quite quick to VTC unclear questions myself, but come on: It's fairly obvious that repetition is the problem here, and it's even mentioned in the question title (even in the first, non-improved version). — Heinzi, Aug 12 '21 at 14:13
@Heinzi I've gone ahead and done a rare thing and acted on the *assumption* that your interpretation is correct. Generally, it is OP's responsibility to provide clear, concise requirements for their Q when asking for a refactor of otherwise-working code... such askers *usually* make similar requests like "make it shorter", "avoid repetition", but which parts you consider 'repetitive' are assumptions. The entire if statement? How the variables are being added? The need for so many incrementing variables? There's a reason Code Review requires full working code samples & thorough explanations. — TylerH, Aug 12 '21 at 14:29
I could at best cut it down to a repition of `if(isMusic) output += "Music "` | IIRC Reflection has some options to get the name of a variable rather then it's value (it is what ArgumentException tends to use), but that would tie the variable name to the (propably) user facing output - bad idea. — Christopher, Aug 15 '21 at 12:24

score 1 · Answer 1 · answered Aug 12 '21 at 13:06

You can use the params keyword:

public static void MyFunc(params string[] vars)

which will automatically translate a call of

MyFunc("a", "b", "c")

into

MyFunc(new string[] {"a", "b", "c"})

Then, you can use the "usual" looping constructs (see below for an example) to loop through your parameter array and avoid repetition of your code. Note that your method will now support an arbitrary number of arguments, which may (or may not) be what you want.

Alternatively, if you only want to support a fixed number of arguments, you can manually "convert" them into an array at the beginning of your method and then loop over those items:

var vars = new string[] { var1, var2, var3 };

for (int i = 0; i < vars.Length; i++)
{
    if (!string.IsNullOrEmpty(vars[i]))
    {
        g.Add("var" + (i + 1) + " operator " + var[i]);
    }
}

score 1 · Answer 2 · 2021-08-12T13:18:15.340

You can refactor like that:

  public static void MyFunc(string var1, string var2, string var3)
  {
    List<string> g = new List<string>();
    process(nameof(var1), var1);
    process(nameof(var2), var2);
    process(nameof(var3), var3);
    void process(string varName, string varValue)
    {
      if ( !string.IsNullOrEmpty(varValue) )
        g.Add(varName + " operator " + varValue);
    }
  }

More details about what you do and what you need to do is required to write something better, thus I just improved the code provided as is.

In the event of an indeterminate list of parameters, there will be a problem to get the name of the variable...

How to check if the parameter of a method comes from a variable or a literal?

But a Dictionary<string, string> can be used:

Dynamic variable in C#?

arne · Answer 3 · 2021-08-12T13:28:39.983

-1

try this solution:

public static void MyFunc(string[] parameters)
{
    List<string> g = new List<string>() { parameters.Where(x => string.IsNullOrWhiteSpace(x) == false).Select(x => $"var{Array.IndexOf(parameters, x)} operator {x}").ToList(); }
}

edited Aug 12 '21 at 13:28

answered Aug 12 '21 at 13:10

arne

123
9

1

Also, that `ToList().ForEach()` is very expensive. It should have been just `.Where().Select().ToList()` – Camilo Terevinto Aug 12 '21 at 13:20
1

But you can't build the string without the ```.ForEach()```. – arne Aug 12 '21 at 13:23
2

That's why I suggested `Select` instead :) `MyFunc(string[] parameters) { parameters.Where(x => !string.IsNullOrWhiteSpace(x)).Select(x => $"var{Array.IndexOf(parameters, x)} operator {x}").ToList(); }` – Camilo Terevinto Aug 12 '21 at 13:25
1

Ah now I understand what you mean :D – arne Aug 12 '21 at 13:26

cherrywoods · Answer 4 · 2021-08-12T13:25:26.867

-1

Put all your strings in an array and use a for loop:

List<string> g;

string[] vars = {var1, var2, var3};
for (int i = 0; i < vars.Length; i++)
{
    if (vars[i] != null && vars[i] != "")
    {
        g.Add( "var " + i + " operator " + vars[i] );
    }
}

edited Aug 12 '21 at 13:25

answered Aug 12 '21 at 13:10

cherrywoods

1,284
1
7
18

1

Will this add "var2 operator somevalue" for second var? – Guru Stron Aug 12 '21 at 13:14

How to avoid repetitive statements in this code?

4 Answers4

Linked

Related