Martijn Kruithof wrote:
Sorry about this one (the ternary conditional). I very rarely use these, and in this case, I was getting "uneccessary nesting" complaints from jikes, eclipse and PMD, so I thought I'd get rid of the nesting, then I decided to rewrite as a ternary as it would remove the nesting problem, I didn't actually want to include it in the patch, so I changed it back (and I didn't think it was in there when I scanned the patch file), because as you rightly say it looks ugly!Conor MacNeill wrote:
Likewise, I do not see converting an if-else to a
ternary conditional makes the code clearer - on the contrary it makes it
more convoluted.
This is my biggest personal gripe with multiple returns in a single method. I can see the value of "quick return", especially in long methods, but in many cases it's an if-else with two returns, one of which doesn't need to be in the else block, you get the situation where to make the code more readable you have to nest a return (else) or you make the code less readable to avoid a reasonable compiler warning. I personally prefer to keep the compiler as happy as possible at the expense of a small amount of readablility, but obvisouly I'll adapt to the general style of the project as needed.
- if (callee != null) {
- return callee.handleInput(buffer, offset, length);
- } else {
- return super.handleInput(buffer, offset, length);
- }would then become
if (callee != null) {
return callee.handleInput(buffer, offset, length);
}
return super.handleInput(buffer, offset, length);Again I agree the garbled ternary should never have been in the patch.
As for variable names, the abbreviations were made solely to prevent name-hiding warnings. I treat all warnings as errors (hence my local copy of Ant doesn't completely compile), and try to eliminate them. As you can see from the patch, most of the time, I renamed the variables to be longer (filesToAdd etc) instead of abbreviating them, as I too prefer readable variable names. For some of them I couldn't come up with a reasonable new name without having something ridiculous, and perhaps I should have left them alone, but I thought I'd get rid of all the name-hiding issues (for the core) in one fell swoop.No, not *that* much but it's not really necessary. As an example, I personally find in the following, the original code is *much* clearer in intent and even in formatting.
- if (callee != null) { - return callee.handleInput(buffer, offset, length); - } else { - return super.handleInput(buffer, offset, length); - } + return (callee != null ? + callee.handleInput(buffer, offset, length) : + super.handleInput(buffer, offset, length) + );
Personally, I primarily use terneraries for situations such as parameter passing, where I need to pass one of two values. In general, I much prefer readable code with good variable names over smaller code with abbrv vrbl nms.
Conor
It seems that the most of my (small) contributions of last week were in vain, c'est la vie. Sorry for wasting time people.
Kev
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
