> 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 <[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
