1
\$\begingroup\$

I was thinking about this after recently reading about the new Null Conditional Operators in C# 6.0. So here is my code that I am currently working on...

    if (!string.IsNullOrWhiteSpace(question2Answer))
    {
        stringbuild.Append($" and ProdUsers = {Regex.Match(question2Answer, @"\d+").Value}");
    }
    if (!string.IsNullOrWhiteSpace(question3Answer))
    {
        stringbuild.Append($" and ProdFullyOnline = '{(int)Enum.Parse(typeof(YesNo), question3Answer, true)}'");
    }
    if (!string.IsNullOrWhiteSpace(question4Answer))
    {
        stringbuild.Append($" and ProdMultiCurrency = '{(int)Enum.Parse(typeof(YesNo), question4Answer, true)}'");
    }
    if (!string.IsNullOrWhiteSpace(question5Answer))
    {
        stringbuild.Append($" and ProdPrice <= {question5Answer}");
    }

As you can see, it is fairly repetitive. So my question, is would there be a better way of checking whether questionXAnswer is null or not and then using the not null value on the string builder. And it would be nice to use the new null conditional operators with C# 6.0, if possible.

The main reason for me posting this however, is to solve the repetition problem if possible. And obviously, any other ways I could improve this would be appreciated too!

\$\endgroup\$
  • 6
    \$\begingroup\$ Hello. My name is Mr Tables. I was wondering if you knew my son, Bobby. (xkcd) \$\endgroup\$ – Dan Pantry Feb 23 '16 at 15:01
  • 5
    \$\begingroup\$ Truth is, we'll need much more context than that snippet to help you restructure your code without making very wild assumptions. Please edit your question to include at least the whole method, ideally the whole class - the fact that you have variables named questionXAnswer is a design smell that we unfortunately cannot address in the current state of the question. Also, please edit your title to tell us what your code is doing, not what you want to improve in it. \$\endgroup\$ – Mathieu Guindon Feb 23 '16 at 15:05
1
\$\begingroup\$

Since you are doing the same operation over and over, you can put your strings in an array or list and use a mapping function.

For example:

//this struct makes it easy to initialize the list of values
public struct Tuple
{
  public String Value;
  public String InterpolatedValue;
  public Tuple(string val, string def)
  {
    Value = val;
    InterpolatedValue = def;
  }
}

//dummy answers to the questions
string question2Answer = "aaa";
string question3Answer = "bbb";
string question4Answer = "";
string question5Answer = "ddd";

//initialize a list of answer/interpolated answer strings
List<Tuple> list = new List<Tuple>()
{
  new Tuple(question2Answer, $" and ProdUsers = {question2Answer}"),
  new Tuple(question3Answer, $" and ProdFullyOnline = '{question3Answer}'"),
  new Tuple(question4Answer, $" and ProdMultiCurrency = '{question4Answer}'"),
  new Tuple(question5Answer, $" and ProdPrice <= {question5Answer}"),
};

//loop over list, checking for non empty answer
//this line and the next takes the place of almost all of your original code
IEnumerable<string> tempList= list.Select(t => (string.IsNullOrWhiteSpace(t.Value) ? string.Empty : (t.DefaultValue)));

//concat results into a single string
string text = string.Join("", tempList);

Console.WriteLine(text);

Outputs: " and ProdUsers = aaa and ProdFullyOnline = 'bbb' and ProdPrice <= ddd"

I had to add some boilerplate code to make the example work, but I think one gets the idea...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.