Object oriented programming is the weapon of choice for many programmers, and when wielded properly, you can often rely on a mix of convention and strong types to make it clear what type of object you’re working with. Sometimes though, you need to check. In a language like Java, you have the instanceof operator, a boolean comparison which answers if obj instanceof SomeClass. Seeing a lot of that in a codebase is a clear code smell.

Sometimes, though, not seeing it is the code smell.

Chris S spotted this pattern repeatedly in their codebase:

  setHasTypeCorporateAccountOrTypeCustomerAccount(hasTypeCorporateAccount() || hasTypeCustomerAccount() || hasTypeCorporateAccountOrTypeCustomerAccount());

This code, inside of the Account class, is calling a set method to set the value of hasTypeCorporateAccountOrTypeCustomerAccount. It determines that based on related boolean values- hasTypeCorporateAccount and hasTypeCustomerAccount, and of course hasTypeCorporateOrTypeCustomerAccount (the value being set).

As you might have guessed, this creates a situation where the boolean flags are used for a weird version of polymorphism elsewhere in the codebase, e.g., if (acct.hasTypeCorporateAccount()) {…}. That’s already ugly, but as implemented, the underlying fields are just boolean values, and the set methods just directly set the values, with no checks. So it’s entirely possible to have hasTypeCorporateAccount() return true, while hasTypeCorporateAccountOrTypeCustomerAccount() return false. And sometimes, that happens, and is the root cause of a bunch of different bugs.

Chris can change the getters to make this behave rationally, but fixing the root cause- the person who wrote this- might be a little harder.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!