HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Getting filtered autolist

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
autolistgettingfiltered

Problem

Is there a better way to write the following code fragment?

I can't have more than 10 conditional statement in my method as it gives cyclomatic complexity.

private List getFilteredAutoList() 
{
    List autoList = getAutoListForCoverage();
    List filteredAutoList = new ArrayList();

    for(AutoDto auto : autoList)
    {
        if("FAQ025".equals((auto.getCarCode()))
                ||"FAQ025".equals((auto.getCarCode()))
                ||"QEF025".equals((auto.getCarCode()))
                ||"QEF037AB".equals((auto.getCarCode()))
                ||"FAQ037AB".equals((auto.getCarCode()))
                ||"FAQ045".equals((auto.getCarCode()))
                ||"QEF045".equals((auto.getCarCode()))
                ||"NBEF028".equals((auto.getCarCode()))
                ||"OPCF028".equals((auto.getCarCode()))
                //20 more OR conditions
                )
        {
            filteredAutoList.add(auto);
        }
    }
    return filteredAutoList;
}


The following two approaches came to my mind:

-
Using HashSet

private List getFilteredAutoList() 
{
    List autoList = getAutoListForCoverage();
    List filteredAutoList = new ArrayList();

    Set carCodeSet = getCarCodeSet();

    for(AutoDto auto : autoList)
    {
        if(carCodeSet.contains(auto.getCarCode()))
        {
            filteredAutoList.add(auto);
        }
    }
    return filteredAutoList;
}

private Set getCarCodeSet()
{
    Set carCodeSet = new HashSet();

    carCodeSet.add("FAQ025");
    carCodeSet.add("QEF025");
    carCodeSet.add("QEF037AB");
    carCodeSet.add("FAQ037AB");
    carCodeSet.add("FAQ045");
    carCodeSet.add("QEF045");
    carCodeSet.add("NBEF028");
    carCodeSet.add("OPCF028");
    //20 more codes to be added

    return carCodeSet;
}


-
Using String contains method

```
private List getFilteredAutoList1()
{
List autoList = getAutoListForCoverage();
List filteredAutoList = new ArrayList();

String carCodeString = "FAQ025,QE

Solution

Use Collection.contains.

That reduces your if statement to the following:

if ( filterCarCodes.contains(auto.getCarCode()) ){
    //stuff
}


You can initialize the Collection once with Arrays.asList, using a String array. Then declare it as a static final class variable and you're pretty much set. An alternative is the HashSet you described - via Arrays.asList you'd have a filled Collection, which is one of the constructors for a HashSet with contents: HashSet(Collection c). You'll pay for the double conversion, but it should be faster than directly using a List.

Code Snippets

if ( filterCarCodes.contains(auto.getCarCode()) ){
    //stuff
}

Context

StackExchange Code Review Q#59103, answer score: 11

Revisions (0)

No revisions yet.