or condition in if condition

Question

i have condition need to be check

if(staffid!=23||staffid!=24||staffid!=25||staffid!=26||staffid!=27||staffid!=29||staffid!=31)
{
  do the  req  thing ..
}

right now i checking the condition like this . is their any better way to write this condition

thank you

Answer 1

Merging several of the other answers (mjv, pasta, Mike Hofer, R. Bemrose) together you will come up with the following code.

1. Use a function to test if the staff ID is valid, that way you only need to change one place.
2. An int array doesn't have a Contains method, so you will need to convert it to an IList (Unless using the extension methods provided in 3.0 in the System.Linq namespace).

As for the code:

if(!isStaffIDValid(staffid))
{
    //do the req thing ..
}

...

Then in either the same class, or more preferably a global class use this code:

public static IList<int> notAllowedIDs = new int[] { 23, 24, 25, 26, 27, 29, 31 };
public static bool isStaffIDValid(int staffID)
{
    return !notAllowedIDs.Contains(staffID);
}

This provides more maintainable code that can be easily updated.

Answer 2

Errr.. isn't that equivalent to:

if (true) { do the req thing... }

Unless staffid can simultaneously be 23 and 24 and 25 and 26 and 27 and 29 and 31.

Imagine 2 cases:

1. staffid = 23
2. staffid != 23

Your statement:

if(staffid!=23 ||
   staffid!=24 ||
   staffid!=25 ||
   staffid!=26 ||
   staffid!=27 ||
   staffid!=29 ||
   staffid!=31)
{
  do the  req  thing ..
}

Case 1 passes the second test (staffid != 24), and case 2 passes the first test (staffid!=23). Since case 1 and case 2 together account for all cases, all values of staffid should pass your test.

Answer 3

Can't imagine what your actual problem is, the statement looks wrong.

If there are lots of "not"s in a complex condition, just convert it to say the contrary. If there is both a if and else section, swap them. If there is no else, put the "not" to the beginning. Your condition looks wrong, just to show what I mean, here is the converted one:

if (staffid == 23 
  && staffid == 24
  && staffid == 25
  && staffid == 26
  && staffid == 27
  && staffid == 29
  && staffid == 31)
{
  //if there was an else block before, it will be here now.
}
else
{
  //do the  req  thing ..
}

Then you can more easily understand the condition, and more easily see that it can't be what you need...

Answer 4

Use a function allowStaff(staffid, "Payment"). Then have all your checking done in one central function allowStaff. This way even if you get a clever idea, you can change it in one place and quicker!

Answer 5

First, what is the data type of staffid? Is it an enum? An int?

Then, the variable's name is sending up a red flag to me. Are you hard-coding behavior for specific individuals or roles into your application when those behaviors/roles may change down the road? You may want to rethink that.

Now, that out of the way, and assuming that staffid is an int:

int[] inValidIds = {23, 24, 25, 26, 27, 29, 31};
if (! ((IList<int>)inValidIds.Contains(staffId)))
{
    // Do stuff;
}

See here.

Answer 6

Assuming you mean:

    if (staffid != 23 && staffid != 24 && staffid != 25 && staffid != 26 && staffid != 27 && staffid != 29 && staffid != 31)
    {
        // Do Stuff
    }

Saw a nice extension method to do this:

public static bool In<T>(this T source, params T[] list)
{
  if(null==source) throw new ArgumentNullException("source");
  return list.Contains(source);
}

so your code would be:

if(!staffid.In(23, 24, 25, 26, 27, 29, 31))
{
  do the  req  thing ..
}

From this answer

Answer 7

I think Chris is on the right track here, but why convert to a list?

public static int[] notAllowedIDs = new int[] { 23, 24, 25, 26, 27, 29, 31 };

// Other code here

if (Array.IndexOf(notAllowedIDs, staffId) < 0)
{
    // do the  req  thing ..
}

Additional notes: Array.IndexOf and List.Contains are both O(n) operations, where n is the number of elements. However, Array.IndexOf saves a conversion from an array to a list.

Array.IndexOf returns the array's lower bound - 1 when the element isn't found, which is -1 for most arrays.

Answer 8

How about just

if(staffid < 23 || staffid == 30 || staffid > 31)
{
    do the  req  thing ..
}

Answer 9

EDIT: your logic is probably wrong. You can use a switch with cases that cascade, e.g.

switch(var)
{
   case foo:
   case bar:
      doIt();
      break;
}

Answer 10

Put your lists of staff id's in a List, and use LINQ to query a list of integers (if staffid not in list).