> 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 <[email protected]>wrote: > On Sun, Sep 1, 2013 at 8:08 PM, David Deutsch <[email protected]> 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 <[email protected]> > > 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 <[email protected] > >> > <mailto:[email protected]>> wrote: > >> > > >> > > >> > On Aug 31, 2013, at 2:00 PM, David Deutsch <[email protected] > >> > <mailto:[email protected]>> 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 > >> > [email protected] <mailto:[email protected]> > >> > http://lists.roundcube.net/mailman/listinfo/dev > >> > > >> > > >> > _______________________________________________ > >> > Roundcube Development discussion mailing list > >> > [email protected] > >> > http://lists.roundcube.net/mailman/listinfo/dev > >> _______________________________________________ > >> Roundcube Development discussion mailing list > >> [email protected] > >> http://lists.roundcube.net/mailman/listinfo/dev > > > > > > > > _______________________________________________ > > Roundcube Development discussion mailing list > > [email protected] > > http://lists.roundcube.net/mailman/listinfo/dev > _______________________________________________ > Roundcube Development discussion mailing list > [email protected] > http://lists.roundcube.net/mailman/listinfo/dev >
_______________________________________________ Roundcube Development discussion mailing list [email protected] http://lists.roundcube.net/mailman/listinfo/dev
