Bug 102719

Description Michael Moser 2005-07-05 05:53:11 EDT

Hi - I am defining me a CComboFieldEditor, i.e. a FieldEditor that provides a 
couple fo predefined values via a drop-down menu plus allows a user to enter 
arbitrary additional values.
To save me some work and stay as close as possible to the spirit of the other 
FieldEditors I considered "stealing" some code from StringFieldEditor.

Doing so, I encountered the below method. It contains not really a bug, but 
some code that puzzled me and probably does not exactly what the author had in 
mind:

    /**
     * Checks whether the text input field contains a valid value or not.
     *
     * @return <code>true</code> if the field value is valid,
     *   and <code>false</code> if invalid
     */
    protected boolean checkState() {
        boolean result = false; //<<<<
        if (emptyStringAllowed) //<<<<
            result = true; //<<<<

        if (textField == null) //<<<<
            result = false; //<<<<

        String txt = textField.getText();

        result = (txt.trim().length() > 0) || emptyStringAllowed; //****

        // call hook for subclasses
        result = result && doCheckState();

        if (result)
            clearErrorMessage();
        else
            showErrorMessage(errorMessage);

        return result;
    }
    
IMHO, the lines marked with "//<<<<" are superfluous, as their effect is 
cancelled (i.e. the "result" is overwritten) in the line marked with "//****".

So one could either simply remove them (except the initial "boolean result;" 
variable declaration of course) or maybe rethink that code, since I believe, 
that these lines were probably initially meant as guard against e.g. 
textField==null - in which case one probably should not call textField.getText
(), etc.

Michael