Removing code duplication and redundancy based on different criteria and priority level

Question

I was wondering if I could get your suggestions on how to remove the code duplication/repetition here.

Any preferred and correct way of doing so?

Summary: This function returns a car's name and accepts the car code as a parameter

public string GetMatchingCar(string carCode)
{
    //CarResults is the list of cars that consists the details of the car as a collection.
    var carResults = new List<Car>
    {
        new Car{Name="AB car", Code="AB", Colors = new List<string>{"blue","green","black"}, Features=new List<string>{"compact","fast","light"} },
        new Car{Name="AC car", Code="AC", Colors = new List<string>{"gray","white","yellow"}, Features=new List<string>{"extended","fast","heavy"} },
        new Car{Name="DE car", Code="DE", Colors = new List<string>{"red","green","purple"}, Features=new List<string>{"sports","light"} },
        //and so on
    };

    //Specifications is a list of specification to choose from that includes color and feature.
    var specifications = new List<Specification>
    {
        new Specification{Color="blue", Feature="heavy"},
        new Specification{Color="red", Feature="light"},
        new Specification{Color="maroon", Feature="compact"},
        new Specification{Color="black", Feature="manual"},
        new Specification{Color="neon", Feature="heavy"},
        //and so on
    }

    //Now, we need to return the car based on the priority of criteria. Say P1, P2 and P3. P1's priority being highest.
    //P1: Code + Color + Feature
    //P2: Code + Feature
    //P3: Feature
    //just for example, priorities can be from P1 - P7

    //Now we have to combine the specification with carCode to check the priorities and return the matching car.

    //TODO: Here, is where I think I am doing wrong by having each for loop for each priority (as priorities can be upto Seven). All the foreach loop won't be executed if a priority is matched and returns a car.
    //TODO: Wondering how could I improve this?

    //priority P1: Code + Color + Feature 
    foreach(var specification in specifications){
        var matchingCarP1 = carResults.FirstOrDefault(x=> x.Code.Equals(carCode) && x.Colors.Contains(specification.Color) && x.Features.Contains(specification.Feature)); 
        if(matchingCarP1 != null) return matchingCarP1.Name;
    }

    //priority P2: Code + Feature
    foreach(var specification in specifications){
        var matchingCarP2 = carResults.FirstOrDefault(x=> x.Code.Equals(carCode) && x.Features.Contains(specification.Feature)); 
        if(matchingCarP2 != null) return matchingCarP2.Name;
    }

    //priority P3: Feature 
    foreach(var specification in specifications){
        var matchingCarP3 = carResults.FirstOrDefault(x.Features.Contains(specification.Feature)); 
        if(matchingCarP3 != null) return matchingCarP3.Name;
    }
    
    //Other priorities

    return string.Empty;
}

Any suggestions or feedback on this would be really helpful and highly appreciated! Thank you!

Answer 1

In my view, this method has too many responsibilies and it is too long. Let's try to apply Single Responsibility Principle of SOLID.

So let's create a method with one goal. The goal or responsibility will be just to get cars:

private List<Car> GetCars() => 
    new List<Car>
    {
        new Car{Name="AB car", Code="AB", Colors = 
            new List<string>{"blue","green","black"}, 
            Features=new List<string>{"compact","fast","light"} },
        new Car{Name="AC car", Code="AC", Colors = 
            new List<string>{"gray","white","yellow"}, 
            Features=new List<string>{"extended","fast","heavy"} },
        new Car{Name="DE car", Code="DE", Colors = 
            new List<string>{"red","green","purple"}, 
            Features=new List<string>{"sports","light"} },
        //and so on
    };

and

private List<Car> GetSpecifications() =>
    new List<Specification>
    {
        new Specification{Color="blue", Feature="heavy"},
        new Specification{Color="red", Feature="light"},
        new Specification{Color="maroon", Feature="compact"},
        new Specification{Color="black", Feature="manual"},
        new Specification{Color="neon", Feature="heavy"},
        //and so on
    }

And it looks like that we can avoid unnecessary iteration of the whole specifications by adding nested if statements:

private string FindMatchingCar(List<Car> cars, 
    List<Specification> specifications) 
{
    foreach(var specification in specifications){
        var matchingCarP1 = carResults.FirstOrDefault(x=> x.Code.Equals(carCode) 
            && x.Colors.Contains(specification.Color) 
            && x.Features.Contains(specification.Feature)); 
        if(matchingCarP1 != null) return matchingCarP1.Name;
        
        var matchingCarP2 = carResults.FirstOrDefault(x=> x.Code.Equals(carCode) 
            && x.Features.Contains(specification.Feature)); 
        if(matchingCarP2 != null) return matchingCarP2.Name;
        
        var matchingCarP3 = carResults.FirstOrDefault(x =>  
            x.Features.Contains(specification.Feature)); 
        if(matchingCarP3 != null) return matchingCarP3.Name;
    }
        
    return string.Empty;
}

And then your method will look like this:

public string GetMatchingCar(string carCode) 
{
    var cars = GetCars();    
    var specifications = GetSpecifications();    
    return FindMatchingCar(cars, specifications);
}