4
\$\begingroup\$

This is simply an extension method which checks if a string str contains all the values in values.

However, there are cases when str might be null, values might be null, or any item in values might be null. I am handling those cases by using the ternary ?:, null-conditional ?. and null-coalesce ?? operators.

Should this piece of code be refactored?

public static bool ContainsAll(this string str, string[] values)
{
    return str == null ? false : values?.All(v => str.Contains(v ?? "")) ?? false;
}
\$\endgroup\$
  • \$\begingroup\$ Isn't it self-explanatory? \$\endgroup\$ – NotADeveloper Jan 29 '17 at 20:43
  • \$\begingroup\$ I cannot pass null to string.Contains(), hence v ?? "" returns v, if non-null, otherwise "". \$\endgroup\$ – NotADeveloper Jan 29 '17 at 20:46
  • \$\begingroup\$ Now it looks much better and I give you +1 for the improvements ;-) \$\endgroup\$ – t3chb0t Jan 29 '17 at 20:54
7
\$\begingroup\$

I like the ternary operator but in this case I wouldn't use it because a single expression that makes sure both variables are valid and then searching for the strings is easier to understand I think.

The v ?? "" isn't very pretty, especially the "" part which actually should be string.Empty. You could get rid of it by filtering the array and skipping the empty and null elements.

public static bool ContainsAll(this string str, params string[] values)
{
    return 
        !string.IsNullOrEmpty(str) && 
        values != null &&
        values
            .Where(x => !string.IsNullOrEmpty(x))
            .All(v => str.Contains(v));
}

If you decorate the second parameter with params keyword then you can just type the values without an array (if you need to):

"text".ContainsAll("t", null);
\$\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.