Ted Husted wrote:
In general, we work by lazy consensus. If no one squawks, we sally forth.
OK, before I sally ;-) I've been thinking a little more about this change. There are two approaches that seem reasonable, one of which minimized changes in the code and one of which minimizes the amount of code that will have to be reviewed in addition to testing as well as the potential for mistakes to creep in later. Guess which I'm leaning toward ;-)
The first approach would be what Niall already proposed: changing the checks for 'null' in BaseHandlerTag and a few other places. As I said, that has the advantage of minimizing the amount of code change.
The second approach would be to modify the attribute setters to convert empty-String arguments to null right away. This has a few advantages:
- it makes it very clear and explicit that empty strings are being thrown away, and the decision of whether or not to ignore empty strings can be made once per attribute, rather than everywhere that attribute value is referenced
- it ensures that no existing checks for null that should be changed into checks for empty-or-null get missed; whereever there's an 'if (x == null) now, the code would remain unchanged
- more importantly, it ensures no checks for null get added later that should be checks for empty-or-null, particularly since the checking semantics could so easily get out-of-sync if the same check is made in more than one place
Basically, I'm suggesting making more changes now (though still very minor ones, just pervasively) to (a) ensure the code adheres to the DRY principle, (b) reduce the chance of the changes being incomplete (through missing checks that would need to be updated with approach 1) and (c) reduces the chance of future mistakes creeping in.
Am I over-selling? ;-) This is the approach I'll take unless I hear an objection.
L. --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
