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.
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.
Exception: return allowed at the start of a function if preconditions are
not met.
Yeah, I guess we are in agreement about that, then ;-)
I will go through my changes and see how I would deal with the "returns on top and bottom" restriction.
We made our choice and we're going to stick with it. Thus use underlines
and no caps. And we're not doing PSR-1 here.
Understood.
I'm wondering what your IDE means by "conceptual errors".
Oh I think it just does that with lowercase class names, since that's so uncommon.
We definitely want if ($sky == 'blue') and not if ('blue' == $sky).
Awesome! :-D
Will move on to the GitHub comments now.
Thanks for all the feedback!
-David
On Sun, Sep 1, 2013 at 7:20 PM, Thomas Bruederli thomas@roundcube.netwrote:
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 morereadable.
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