Community
Participate
Working Groups
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
I suggest the following modification (at least that's how I implemented my CComboFieldEditor and that seems to work): protected boolean checkState() { boolean result; if (textField == null) { result = false; } else { 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; }
changing prio and status to conform to plat-ui bug guidelines
As per http://wiki.eclipse.org/Platform_UI/Bug_Triage_Change_2009
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.