On Thu, Sep 5, 2013 at 12:11 PM, David Deutsch skoremail@gmail.com wrote:
I really hate negated expressions and mid-block returns but this is not my code I just wrote some plugins to fit RC in my environment.
The way I'm applying them right now, I don't particularly enjoy them either, but I do think they're the lesser of two evils. As discussed before - what you want to have, in the end, is a separation into functions. I think disentangling those large if statements is a good first step towards that.
OK, after long discussions here and in the pull requests on github, can we try to conclude where that refactoring is supposed to go?
For what we have seen and discussed, here's what I would like to happen and not to happen:
- no intermediate returns but clear if/elseif/else clauses. If the
individual clauses exceed a certain number of lines (let's say, 20 lines) they should become individual functions/methods.
- allow immediate returns at the beginning of a function when
pre-conditions are not met.
- only re-arrange blocks of variable assignments or function call
blocks with additional lines where they're really hard to understand.
e.g. these should not be torn apart with blank lines:
$rcmail = rcmail::get_instance();
$identity = $rcmail->user->get_identity();
$identities_level = intval($rcmail->config->get('identities_level', 0));
no one-line if blocks without braces.
all if/for/while blocks should get braces.
else if / else clauses start on a new line. Not "} else {"
object/array function arguments should start on the same line as the
function call. That saves an indentation level.
Good: dialog.dialog({ ... });
$this->api->output->button(array( ... ));
Bad: dialog.dialog( { ... } );
$this->api->output->button( array( ... ) );
- Javascript: opening brace for anonymous functions are on the same
line. No space between 'function' and brackets.
May I ask you to re-do or update the pending pull requests according to this suggestion? And in order to save your precious time, you should wait for approval of the first pull request before moving on.
Thanks a lot for you efforts and all the inputs!
Regards, Thomas