Problem
I have a predicate builder that initially evaluates to false
. Because of this, the very next expression should be a logical Or
. But if I need a logical And
I need to keep track of a boolean value. And this boolean value hasPassedFirstCheck
is the thing I want to get rid of.
var cars = PredicateBuilder.False<Cars>();
var hasPassedFirstCheck = false;
if (filterObject.CarTypes.Contains(CarTypes.Truck))
{
if (sf.ServiceTypes.Count < 2 && !hasPassedFirstCheck)
cars = cars.And(x => x.CarType== CarType.DJ);
else
cars = hasPassedFirstCheck
? cars.Or(x => x.CarType== CarType.DJ)
: cars.And(x => x.CarType== CarType.DJ);
hasPassedFirstCheck = true;
}
if (filterObject.CarTypes.Contains(CarTypes.SUV))
{
if (condition && !hasPassedFirstCheck)
cars = cars.And(x => x.CarType == CarType.SUV);
else
cars = hasPassedFirstCheck
? cars.Or(x => x.CarType == CarType.SUV)
: cars.And(x => x.CarType == CarType.SUV);
hasPassedFirstCheck = true;
}
if (filterObject.CarTypes.Contains(CarTypes.Limo))
{
if (condition && !hasPassedFirstCheck)
cars = cars .And(x => x.CarType == CarType.Limo);
else
cars = hasPassedFirstCheck
? cars.Or(x => x.CarType== CarType.Limo)
: cars.And(x => x.CarType== CarType.Limo);
hasPassedFirstCheck = true;
}
if (filterObject.CarTypes.Contains(CarTypes.Race))
{
if (condition && !hasPassedFirstCheck)
cars = cars.And(x => x.CarType == CarType.Race);
else
cars = hasPassedFirstCheck
? cars.Or(x => x.CarType == CarType.Race)
: cars.And(x => x.CarType == CarType.Race);
hasPassedFirstCheck = true;
}
This also isn’t really OOP. Any suggestion to refactor this is really appreciated.
Solution
If your condition
if false then none of your if
‘s executes. If it’s true then condition && !hasPassedFirstCheck
is always false
, so your code simplifies to this:
var cars = PredicateBuilder.False<Cars>();
if (condition)
{
// I guess you have a typo here as both expressions are same
cars = sf.ServiceTypes.Count < 2 ? cars.And(x => x.CarType == CarType.Truk)
: cars.And(x => x.CarType == CarType.Truk);
cars = cars.Or(x => x.CarType == CarType.SUV ||
x.CarType == CarType.Limo ||
x.CarType == CarType.Race);
}
And further reduced, but less readable:
var cars = !condition ? PredicateBuilder.False<Cars>() :
(sf.ServiceTypes.Count < 2 ?
cars.And(x => x.CarType == CarType.Truk) :
cars.And(x => x.CarType == CarType.Truk))
.Or(x => x.CarType == CarType.SUV ||
x.CarType == CarType.Limo ||
x.CarType == CarType.Race);
- You seem to have a typo in your
CarType
enum, it should probably beTruck
. -
cars
is initiallyFalse
hence you can get rid of theAnd
conditions because they will never evaluate to true anyway. So for example this makes this whole block useless:if (condition) { cars = sf.ServiceTypes.Count < 2 ? cars .And(x => x.CarType== CarType.Truk) : cars .And(x => x.CarType== CarType.Truk); hasPassedFirstCheck = true; }
Update: Let’s walk through the code a bit.
This is how it starts:
var cars = PredicateBuilder.False<Cars>();
var hasPassedFirstCheck = false;
if (filterObject.CarTypes.Contains(CarTypes.Truck))
{
if (sf.ServiceTypes.Count < 2 && !hasPassedFirstCheck)
cars = cars.And(x => x.CarType== CarType.DJ);
else
cars = hasPassedFirstCheck
? cars.Or(x => x.CarType== CarType.DJ)
: cars.And(x => x.CarType== CarType.DJ);
hasPassedFirstCheck = true;
}
The initial predicate evaluates to false
and hasPassedFirstCheck
is false
as well. Entering the first block this means !hasPassedFirstCheck
is always true
and can be omitted and hasPassedFirstCheck
is always false
hence the Or
path is never taken. Given all of that the code boils down to:
if (filterObject.CarTypes.Contains(CarTypes.Truck))
{
if (sf.ServiceTypes.Count < 2)
cars = cars.And(x => x.CarType== CarType.DJ);
else
cars = cars.And(x => x.CarType== CarType.DJ);
hasPassedFirstCheck = true;
}
Obviously the check if (sf.ServiceTypes.Count < 2)
is redundant because you do the same thing in either case. So it can be further simplified to:
if (filterObject.CarTypes.Contains(CarTypes.Truck))
{
cars = cars.And(x => x.CarType== CarType.DJ);
hasPassedFirstCheck = true;
}
However the cars
predicate is false
at that point and false && Something
evaluates to false
. Hence the And
condition has no effect on the result and the block can be simplified into:
if (filterObject.CarTypes.Contains(CarTypes.Truck))
{
hasPassedFirstCheck = true;
}
Given all of this I’m not sure that
- The code you supplied is actually the code you use.
- That the code is actually doing what you expect it to do in the first place.