> A proper and predictable process flow is something I learned to appreciate during my years as a software developer.

Sure, although I don't understand how my approach makes it any less predictable. All I have done is fan out the if/elseif structures and, arguably, that makes it easier to check through them. Maybe changing it to no-oneliner-ifs will make this a little clearer for you, but the return statements really stand out when you scroll through the code - whether or not they are wrapped in if blocks or individually.

> What's wrong about if clauses with proper indenting? These can even be collapsed with a sexy IDE.

Nothing wrong with them. It's just: Do you wrap the entire content of a function in an if or do you make a negated if -> return statement? The first doesn't scale, the second makes things more readable - that was really all I was trying to convey.

> If the structure gets too complicated, individual blocks should maybe be moved to individual functions with sensible names.

Precisely my point ;-) What I'm doing right now is cleanup to make the existing functions easier to read. If adding functions was on the table, my work would be very different, of course.

-David


On Mon, Sep 2, 2013 at 9:13 PM, Thomas Bruederli <thomas@roundcube.net> wrote:
On Sun, Sep 1, 2013 at 8:08 PM, David Deutsch <skoremail@gmail.com> wrote:
>> And what's this 80 characters discussion anyways? Who still has a screen
>> that only fits 80 characters? Line breaks should be added where it makes
>> sense but please don't take these 80c as the holy limit.
>
> Yeah, well, as I've said: It's really more of a personal preference of mine.
> For my own code, I have found that trying to see 80 lines as a semi-holy
> limit can help to force simplify my approach.
>
> It's obviously no longer about small screens (also my biggest pet peeve
> about people dismissing tabs as indents because some exotic strawman IDEs
> can't deal with them properly).
>
>
>> Single exit point of a function! Don't spread return statements all over.
>
> Would need a clarification here. There are tons and tons of examples in the
> original code where this is not the case. My approach is "Handle corner
> cases first, then the big pile of code" - so with the examples, get a couple
> of returns out of the way, then do what the function should usually do.

I didn't say that our codebase is free of these but I'd not want more
intermediate returns to be added but rather getting rid of them. A
proper and predictable process flow is something I learned to
appreciate during my years as a software developer.
>
> As an extreme example: You could start a function with an if structure that
> checks whether all arguments of the function are the correct type. But you
> wouldn't want that to create a five level indent - you want to handle them
> in a structure like the way I set them up and then move on to the actual
> code.

What's wrong about if clauses with proper indenting? These can even be
collapsed with a sexy IDE. If the structure gets too complicated,
individual blocks should maybe be moved to individual functions with
sensible names.

>> Exception: return allowed at the start of a function if preconditions are
>> not met.
>
> Yeah, I guess we are in agreement about that, then ;-)

Great!

> Will move on to the GitHub comments now.

Aye!

~Thomas


> On Sun, Sep 1, 2013 at 7:20 PM, Thomas Bruederli <thomas@roundcube.net>
> wrote:
>>
>> David Deutsch wrote:
>> > Thanks.
>> >
>> > And find them I did!
>> >
>> > It's just that Thomas said earlier they were the standard in the
>> > codebase
>> > and me changing them would confuse existing developers. So I remain
>> > unconvinced there is what I'm saying... ;-)
>>
>> I obviously got your intentions wrong and I'm sorry about this.
>> We definitely want if ($sky == 'blue') and not if ('blue' == $sky).
>>
>> ~Thomas
>>
>> >
>> >
>> > On Sat, Aug 31, 2013 at 10:43 PM, Cor Bosman <cor@xs4all.nl
>> > <mailto:cor@xs4all.nl>> wrote:
>> >
>> >
>> >     On Aug 31, 2013, at 2:00 PM, David Deutsch <skoremail@gmail.com
>> >     <mailto:skoremail@gmail.com>> wrote:
>> >
>> >>     Another possibly neat comparison:
>> >>
>> >>     Original:
>> >>
>> >> https://github.com/roundcube/roundcubemail/blob/master/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php#L206
>> >>     Cleanup:
>> >>
>> >> https://github.com/daviddeutsch/roundcubemail/blob/37167c5ce1c00cb4f42b7f59a9ff56b81b3cd874/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php#L219
>> >>
>> >>     The code may be /slightly/ less DRY, but, I find, a lot more
>> >> readable.
>> >
>> >     Definitely more readable.
>> >
>> >>
>> >>     I must also say that so far, I find very few yoda conditions! ;-)
>> >
>> >     Find them you will.
>> >
>> >     Cor
>> >
>> >
>> >     _______________________________________________
>> >     Roundcube Development discussion mailing list
>> >     dev@lists.roundcube.net <mailto:dev@lists.roundcube.net>
>> >     http://lists.roundcube.net/mailman/listinfo/dev
>> >
>> >
>> > _______________________________________________
>> > Roundcube Development discussion mailing list
>> > dev@lists.roundcube.net
>> > http://lists.roundcube.net/mailman/listinfo/dev
>> _______________________________________________
>> Roundcube Development discussion mailing list
>> dev@lists.roundcube.net
>> http://lists.roundcube.net/mailman/listinfo/dev
>
>
>
> _______________________________________________
> Roundcube Development discussion mailing list
> dev@lists.roundcube.net
> http://lists.roundcube.net/mailman/listinfo/dev
_______________________________________________
Roundcube Development discussion mailing list
dev@lists.roundcube.net
http://lists.roundcube.net/mailman/listinfo/dev