And that matters because... ? The problem is that whenever you change the framework there may be unintended side-effects because the way you think it should be used may not be the way it is being used (or even the way it was originally intended to be used).
Even if you test a lot there is always the risk of this. So, the point is that you shouldn't be surprised by things like this. Yes, it is disappointing that people fail to use obvious things like an isEmpty method. Whatever the case, if you change how the code operates you ALWAYS risk these sorts of issues. It's still to say the problem was caused by how the class was being used... maybe convenient, but not really correct or useful. Yes, if only everyone would always develop things the way that I think they should be developed! Well, if that were the case for me, OFBiz would be a lot different right now... :) -David On Feb 2, 2010, at 11:28 AM, Adrian Crum wrote: > I looked through some example uses of FlexibleStringExpander and there is a > lot of inconsistency. Some client code expects getOriginal() to always return > a String - it doesn't check for null. Other code checks for null. > > The whole point of the null instance was to eliminate all of the checking for > this or that. Just use it as if it contains something. If it's empty it will > do nothing. > > -Adrian > > David E Jones wrote: >> There's the fun of changing the framework... >> -David >> On Feb 2, 2010, at 10:40 AM, Adrian Crum wrote: >>> Okay, I found the problem and committed a fix. We really need to go through >>> the framework and fix the code that uses FlexibleStringExpander in >>> inappropriate ways. >>> >>> FlexibleStringExpander.getInstance will ALWAYS return an instance, so there >>> is no need to check if an instance is equal to null. >>> >>> Checking FlexibleStringExpander.getOriginal() for null or empty is bad >>> coding style. There is an isEmpty() method for that. >>> >>> -Adrian >>> >>> Adrian Crum wrote: >>>> Hans, >>>> How is that commit affecting UI labels? >>>> -Adrian >>>> Hans Bakker wrote: >>>>> Hi this seems to be caused by r904592 >>>>> >>>>> to revert this change: >>>>> svn merge http://svn.apache.org/repos/asf/ofbiz/trunk -r904592:904591 >>>>> >>>>> Regards, >>>>> Hans >>>>> >>>>> >>>>> On Mon, 2010-02-01 at 23:07 -0800, Scott Gray wrote: >>>>>> After taking an update today, a lot of form widget field labels seem to >>>>>> be missing from various screens. If any committers are aware of any >>>>>> changes they may have made which could have impacted this please take a >>>>>> look and see if it's related to your work. >>>>>> >>>>>> Here's an example: >>>>>> https://localhost:8443/catalog/control/FindProductConfigItems (only one >>>>>> of the 3 search option fields are presenting a field label) >>>>>> Here's another: https://localhost:8443/accounting/control/findPayments >>>>>> >>>>>> Thanks >>>>>> Scott >>>>>> >>>>>> HotWax Media >>>>>> http://www.hotwaxmedia.com >>>>>> >>>>>>
