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
