Code review comment for lp:~pbeaman/akiban-persistit/fix-1073357-Value-getType

Revision history for this message
Yuval Shavit (yshavit) wrote :

I mentioned this in person to Peter, but it'd be nice to rename skipNull to isNullSkip or such. That way, the two methods would appear next to each other in IDEs with autocomplete. Not only would that be convenient, but it would also serve as a warning to anyone who (as many people do) navigates an API via autocomplete. Such a programmer would see these two similarly-named methods, and hopefully think, "hm, I should read what the distinction is."

As a slight variant, what about two overloads of isNull? One is isNull(boolean skipIfNull), the other is isNull() { return isNull(false); }. I merely mention that as a way of getting the same benefits (easy to find, autocomplete-friendly, yet consistent behavior for isNull() as compared to the other isXxx methods) with a bit less of linguistic hoop-jumping.

« Back to merge proposal