CODEWITHSUNDEEP
c#
Vegar
Vegar

Reputation: 12898

Checking valid combination of items in list

I have a list of items that I need to validate. The list can contain any number of items of the type A, B and C, but before the list can be saved, it must confirm to the following rules:

  1. If you have A, you need either B or C
  2. If you have B, you need A

I have ended up with the following code (saudo code):

bool IsListValid()
{
  var a = list.ContainsAny(A);
  var b = list.ContainsAny(B);
  var c = list.ContainsAny(C);

  if (!a && !b)
    return true;

  if (a && (b || c)
    return true;

  return false;
}

I don't like this code.
1. The use of Any three times in a row will potentially iterate the list three times
2. The if's doesn't look good to me.

Of cause it would be better with different variable names and by extracting the test into methods with good names, but I think there are better ways of solving this entirely. I'm just not sure how...

Any tips?

Upvotes: 4

Views: 211

Answers (3)

Tim Schmelter
Tim Schmelter

Reputation: 460108

I would use a simple loop, it's both, comprehensible and efficient.

bool containsA = false, containsB = false, containsC = false;
for (int i = 0; i < list.Count; i++)
{
    Type itemType = list[i].GetType();
    if (!containsA) containsA = itemType == typeof(A);
    if (!containsB) containsB = itemType == typeof(B);
    if (!containsC) containsC = itemType == typeof(C);
    if (containsA && (containsB || containsC)) return true;
}
return (!containsA && !containsB);

Upvotes: 3

csteinmueller
csteinmueller

Reputation: 2487

var hasA = list.Any(x => x.GetType() == typeof(A));
var hasB = list.Any(x => x.GetType() == typeof(B));
var hasC = list.Any(x => x.GetType() == typeof(C));

//If you have A, you need either B or C
// A AND (B xor C)
if (hasA && (hasB ^= hasC))
   return true;

//If you have B, you need A
if (hasB && hasA)
   return true;

   return false;

Upvotes: -1

Adam V
Adam V

Reputation: 6356

If it's so important that you only go through the list once, you could do this:

bool a = false;
bool b = false;
bool c = false;
foreach(var x in list)
{
  if (x is A) a = true;
  if (x is B) b = true;
  if (x is C) c = true;
}

But I'd leave it as it is. If profiling later shows that this code is becoming a bottleneck, you can revisit it then.

As for the if's, that looks fine to me. As long as A, B and C are properly named (something like hasNotifyEmail or needsResponse, something that explains why you want them to work with the specified rules) then it should be easy for others to understand.

Upvotes: 2

Related Questions