CodeSOD: Hanging By a String

We all know that truth is a flexible thing, that strict binaries of true and false are not enough.

Dana's co-worker knew this, and so that co-worker didn't use any pidling boolean values, no enums. They could do one better.

Now, we're missing a lot of the code, but the pieces Dana shared with us are enough to get the picture"

 public CustomerRecord fetchNextCustomer() { //" String yesNoString = String.valueOf(BusinessDAO.custFlagIsSet()); if(yesNoString.equalsIgnoreCase("true")) yesNoString="Y"; //" and later in this same method " if (yesNoString.equalsIgnoreCase("Y")) { //set a flag on the customer } //" }

True, false, "Y", "N", it's all the same thing, yes? But how does this code actually get used?

 public Vector<Records> getCustomers() { //" String a = String.valueOf(BusinessDAO.custFlagIsSet()); if (a.equalsIgnoreCase("TRUE")) { while(true) { CustomerRecord aCustomer = fetchNextCustomer(); if (null != aCustomer) { records.add(aCustomer); } else { break; } } } else { while(true) { CustomerRecord aCustomer = fetchNextCustomer(); if (null != aCustomer) { records.add(aCustomer); } else { break; } } } return records; }

That's an excellent use of if statements. They're future proofed- if they ever do need to branch on that flag, they're already done with that.

But what about that custFlagIsSet code? What on Earth is that doing?

 public String custFlagIsSet() { BusinessConfig domainObject; try { domainObject = getDomainObject; } catch (Exception e) { logger.error("",e); } boolean isFlag = domainObject.custFlagIsSet(); String isFlagString = String.valueOf(isFlag) ; return isFlagString; }

Obviously, what we're seeing here is a low-level API- that domainObject- being adapted into a more abstract API. Where that low-level API only uses little booleans, our custFlagIsSet object makes sure it only ever exposes strings- a much more flexible and robust datatype. Now, we can see some of the history in this code- before that custFlagIsSet modernized the underlying API, the other methods still needed to use String.valueOf, just in case a boolean accidentially came back.

If you say it carefully, it almost sounds like it makes sense. Almost.

[Advertisement] Manage IT infrastructure as code across all environments with Puppet. Puppet Enterprise now offers more control and insight, with role-based access control, activity logging and all-new Puppet Apps. Start your free trial today!