Predicate builder [closed]

Question

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.

Answer 1

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);

Answer 2

1. You seem to have a typo in your CarType enum, it should probably be Truck.

2. cars is initially False hence you can get rid of the And 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

1. The code you supplied is actually the code you use.
2. That the code is actually doing what you expect it to do in the first place.