Remove Repeated Code

Question

My Question is-

I have two string variables site_inclusion and site_exclusion. If site_inclusion has a value, then I don't care what values site_exclusion contains. That is to say that site_inclusion overrides site_exclusion. If, however, site_inclusion is null and site_exclusion has a value, then I want to examine site_exclusion.

To be more precise:

1. If site_inclusion and site_exclusion are both null then set useTheSynthesizer as true;
2. If site_inclusion is not null and it matches with the regexPattern then set useTheSynthesizer as true. And I don't care what values are there in site_exclusion.
3. If site_inclusion is null and site_exclusion is not null and site_exclusion does not match the regexPattern then set useTheSynthesizer to true.

I wrote the below code but somehow I think, I am repeating some stuff here in the if/else loop. Any code improvements will be appreciated that fulfill my conditions.

String site_inclusion = metadata.getSiteInclusion();
String site_exclusion = metadata.getSiteExclusion();

// fix for redundant data per site issue
if(site_inclusion != null && site_inclusion.matches(regexPattern)) {
    useTheSynthesizer = true;
} else if(site_exclusion != null && !(site_exclusion.matches(regexPattern))) {
    useTheSynthesizer = true;
} else if(site_inclusion == null && site_exclusion == null ) {
    useTheSynthesizer = true;
}

Answer 1

1. You don't really need the last null test.
2. I (personally) find it poor style to do an if(test == true) flag = true statement. You can simply say flag = test.

My recommendation would be:

if(site_inclusion != null)
{
    useTheSynthesizer = site_inclusion.matches(regexPattern);
}
else if(site_exclusion != null)
{
    useTheSynthesizer = ! site_exclusion.matches(regexPattern);
}
else
{
    useTheSynthesizer = true;
}

You could also do it in a oneliner:

useTheSynthesizer = site_inclusion != null ? site_inclusion.matches(regexPattern) : (site_exclusion != null ? ! site_exclusion.matches(regexPattern) : true);

But I find that sort of obnoxious to read.

(Note, I made the assumption that useTheSynthesizer was otherwise false. This isn't explicit in your code or explanation, but I think this assumption was safe.)

Answer 2

I would do it like this:

    boolean useTheSynthesizer;

    if (siteInclusion == null && siteExclusion == null) {
        useTheSynthesizer = true;
    }
    else if (siteInclusion == null) {
        useTheSynthesizer = ( ! siteExclusion.matches(regexPattern) );
    }
    else {
        useTheSynthesizer = siteInclusion.matches(regexPattern);
    }

I also removed the underscores from your variable names, since they do not fit the java naming conventions (and they're hideous IMO).