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/l... Cleanup:
https://github.com/daviddeutsch/roundcubemail/blob/37167c5ce1c00cb4f42b7f59a...
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