-1

I have this piece of code:

if (filter != RECENT &&
    filter != TODAY &&
    filter != WEEK &&
    filter != MONTH &&
    filter != ALLTIME)
{
    filter = RECENT;
}

Note that filter is a string and it's compared against const string types. Is there any way to do this inline and have it be more readable? Doing it with the ternary operator doesn't make it much better since I still have to repeat filter != XXXXX

filter = filter != RECENT &&
         filter != TODAY &&
         filter != WEEK &&
         filter != MONTH &&
         filter != ALLTIME ? RECENT : filter;

and this clearly doesn't work

filter = filter != RECENT && TODAY && WEEK && MONTH && ALLTIME ? RECENT : filter;

Is there any prettier way (prettier == all of the logic must be in a single line of code) to do this comparison? More specifically to prevent filter != XXXXX repetition.

Note that performance is not my primary concern for this question.

mjwills
  • 23,389
  • 6
  • 40
  • 63
bezbos.
  • 1,551
  • 2
  • 18
  • 33
  • 1
    Depends on what `Filter` actually is. Is it an enum? If so, is it an enum with a `[Flags]` attribute? –  Oct 27 '18 at 10:24
  • @elgonzo `filter` is just a `string`. It's being compared against constants, eg. `const string RECENT = "recent";`. – bezbos. Oct 27 '18 at 10:26
  • 3
    You could put those strings perhaps in an array and use Linq. If you then further create/use a static method -- or better an extension method -- with a `params string[] ...` argument (the `params string[]` argument would be the string array i talked about), you can simplify your testing to something like `filter.IsNot(Filter.RECENT, Filter.TODAY, ...)` –  Oct 27 '18 at 10:29
  • 3
    Make an array of your constants and then `!array.Contains(filter)` or consider using enum as elgonzo suggested. We don't know your application but from what we know it could be a better practice than comparing strings. – just-my-name Oct 27 '18 at 10:31
  • 1
    True, suggesting Linq in this scenario here might perhaps be a bit overkill; functions provided by the array type (such as _Contains_) might be all you need... –  Oct 27 '18 at 10:37
  • @just-my-name `filter` is a `string` parameter passed to a method. Making enums would create more work for me. I have used your `...Contains(filter)...` solution and it works nicely. – bezbos. Oct 27 '18 at 10:37
  • And why would inline be more readable? What’s wrong with the current solution’s readability? Is this truly something worth “optimizing” in any way based on what seems clearly opinionated code aesthetics? The only issue could be code repetition but that’s easily solved. – InBetween Oct 27 '18 at 11:10
  • @just-my-name Please answer my question as your comment solved my issue. I will pick it as the "answer" and give you an upvote. `filter = new[]{RECENT, TODAY, WEEK, MONTH, ALLTIME}.Contains(filter) ? filter : RECENT;` solved my problem. – bezbos. Oct 27 '18 at 11:21
  • Possible duplicate of [Multiple string comparison with C#](https://stackoverflow.com/a/6034553/34092) – mjwills Oct 27 '18 at 13:08

2 Answers2

1

I Prefer create an extension method .

  public static bool NotIn(this string filter , params string[] valuesToCompare)
    {
        var result = true;
        foreach (var item in valuesToCompare)
        {
            if (filter == item) return false;
        }
        return result;
    }

and use like

if( filter.NotIn("RECENT", "TODAY ", "WEEK ", "MONTH", "ALLTIME"))
  {
     filter = "RECENT";
  }
Gaurav Moolani
  • 342
  • 1
  • 3
  • 12
0

Note this answer was written before the OP clarified that they wanted only single line solutions.

Using an HashSet

Case-sensitive comparison:

var filters = new HashSet<string>(new[] { RECENT, TODAY, WEEK, MONTH, ALLTIME });

if (!filters.Contains(filter))
{
    filter = RECENT;
}

Case-insensitive comparison:

var filters = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
filters.UnionWith(new[] { RECENT, TODAY, WEEK, MONTH, ALLTIME });

if (!filters.Contains(filter))
{
    filter = RECENT;
}

Using an array of strings

Case-sensitive comparison:

string[] filters = {RECENT, TODAY, WEEK, MONTH, ALLTIME};

if (!filters.Contains(filter))
{
    filter = RECENT;
}

Case-insensitive comparison:

string[] filters = {RECENT, TODAY, WEEK, MONTH, ALLTIME};

if (!filters.Contains(filter, StringComparer.OrdinalIgnoreCase))
{
    filter = RECENT;
}

EDIT

The ternary operator ?: could be used instead of the if statement, but IMO that would make the code less readable. Also, from a debugging perspective, it's easier to set a breakpoint inside the if statement to check if the array contains the filter, as opposed to setting the breakpoint using the operator ?::

Breakpoints

mjwills
  • 23,389
  • 6
  • 40
  • 63
Rui Jarimba
  • 11,166
  • 11
  • 56
  • 86