-1
\$\begingroup\$

I have this Java class which checks if the instance is not blank and matches dev/qa/stg/staging/prod and hostnameVerification is true then getHostnameVerifierForInstance returns a customized HostnameVerifier.

public class SSLConfig {
    private static final String PROD_INSTANCE = "Prod";
    private static final String STG_INSTANCE1 = "Staging";
    private static final String STG_INSTANCE2 = "Stg";
    private static final String QA_INSTANCE = "QA";
    private static final String DEV_INSTANCE = "DEV";

     private static HostnameVerifier getHostnameVerifierForInstance(String instance, String hostnameVerification) {
        LOGGER.info("HostnameVerifier For Instance = {}, hostnameVerification = {}",instance,hostnameVerification);
        HostnameVerifier hostnameVerifier = new HostnameVerifier() {
            public boolean verify(String hostname, SSLSession session) {
                return true;
            }
        };
        if(StringUtils.isNotBlank(instance)) {
            if ((instance.trim().toLowerCase().startsWith(PROD_INSTANCE.toLowerCase()) || instance.trim().toLowerCase().startsWith(STG_INSTANCE1.toLowerCase()) || instance.trim().toLowerCase().startsWith(STG_INSTANCE2.toLowerCase()) || instance.trim().toLowerCase().startsWith(QA_INSTANCE.toLowerCase()) || instance.trim().toLowerCase().startsWith(DEV_INSTANCE.toLowerCase())) && StringUtils.isNotBlank(hostnameVerification) && hostnameVerification.trim().toLowerCase().equalsIgnoreCase("true")) {
                hostnameVerifier = new HostnameVerifier() {
                    public boolean verify(String hostname, SSLSession session) {
                        DefaultHostnameVerifier verifier = new DefaultHostnameVerifier();
                        return verifier.verify(hostname, session);
                    }
                };
            }
        }
        return hostnameVerifier;
    }
}

Can this method be refactored/improved to a better version?

\$\endgroup\$
  • \$\begingroup\$ Everything in this class is private? How do you use it then? \$\endgroup\$ – 200_success Apr 27 '17 at 4:08
  • \$\begingroup\$ If this is just an excerpt, then what else is in the SSLConfig class? \$\endgroup\$ – 200_success Apr 27 '17 at 4:09
  • \$\begingroup\$ toLowerCase().equalsIgnoreCase("true") use either or. \$\endgroup\$ – t3chb0t Apr 27 '17 at 4:22
1
\$\begingroup\$

If you're going to use toLowerCase() why wouldn't you define all your constants as lowercase to begin with, so that you don't need to call toLowerCase() on the constants for every comparison.

It also looks like you want to use an always-true verifier for your internal environments, and the default for all others. Wouldn't it be safer to reverse your logic (make the DefaultHostnameVerifier your default, and change it if it's one of your environments), so that if there's a flaw in your logic, you don't return an always-true verifier by accident.

Generally, in this kind of scenario, returning the safer alternative by default and only returning a less-safe option when specific conditions pass is going to stand up better

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.