Morning Nikita,
All good points, that it's hard to refute.
However, right now, I know where all variables that are going to be used
are declared, no matter the size of the function or it's complexity. If not
at the very top, eyes get good at scanning for blocks.
What I don't want is to have to scan already complicated code to find out
when a variable was declared.
We need to have rules of thumb, and unless someone can defend their use
rigorously, I would say the rule of thumb that code shouldn't be mixi ought
to be followed. At least it ought to be followed in the top layer of PHP
(anything inside a PHP|ZEND_FUNCTION and such).
A defence of elegance is legitimate, so long as their use does genuinely
increase elegance. Such examples exist, but I think more examples of mixi
code being harder to follow exist ...
Cheers
Joe
On Sat, Nov 12, 2016 at 1:06 PM, Nikita Popov <nikita....@gmail.com> wrote:
> On Sat, Nov 12, 2016 at 1:40 PM, Joe Watkins <pthre...@pthreads.org>
> wrote:
>
>> Morning,
>>
>> > okay, I'm only really interested in declarations mixed with code
>>
>> Not sure if serious ... but I will harass you to change code that is mixi
>> [1].
>>
>> I think actually disallowing mixing lends some readability and uniformity
>> to the code in php-src, that I would hate to see disappear ...
>>
>> Cheers
>> Joe
>>
>
> Depends on the situation.
>
> Very commonly this arbitrary restriction requires me to rewrite code into
> a way that is objectively worse, just to avoid repeating initializations.
> Typical example: C99 allows me to write nice code like this:
>
> void foo(foo_t arg) {
> if (!foo_valid(arg)) {
> return;
> }
>
> bar_t bar = get_bar(arg);
> if (!bar_valid(bar)) {
> return;
> }
>
> // Lots of variables here for internal computation
> type1 a = ...;
> type2 b = ...;
> type3 c = ...;
> type4 d = ...;
> }
>
> The important part is that I can easily do an early-return. C89 instead
> forces me to do either this:
>
> void foo(foo_t arg1, bar_t arg2) {
> bar_t bar;
> type1;
> type2;
> type3;
> type4;
>
> if (!foo_valid(arg)) {
> return;
> }
>
> bar = get_bar(arg);
> if (!bar_valid(bar)) {
> return;
> }
>
> // Lots of variables here for internal computation
> a = ...;
> b = ...;
> c = ...;
> d = ...;
> }
>
> Which I find incredibly ugly, as its repetitive and declarations are far
> removed from their source.
>
> Alternatively what I can write is this:
>
> void foo(foo_t arg) {
> if (foo_valid(arg)) {
> bar_t bar = get_bar(arg);
> if (bar_valid(bar)) {
> // Lots of variables here for internal computation
> type1 a = ...;
> type2 b = ...;
> type3 c = ...;
> type4 d = ...;
> }
> }
> }
>
> This is also ugly, because it creates deeply nested code. To avoid code
> that nests many levels deep, what we often do instead (I'm sure you've seen
> this often enough in the php-src codebase) is to instead create huge if
> conditions. You know, the kind where the condition has 6 lines and contains
> assignments on lines 3 and 5. This is part of the reason.
>
> That's one half of the problem. C89 makes early-returns ugly, while I
> think it is nowadays universally recognized that early-returns are
> preferable over deeply nested code.
>
> The other half of the problem is that, as a rule, the lifetime of a
> variable should be minimal to avoid misuse. If you have code like this
>
> type var;
> if (xyz) {
> // ...
> } else {
> var = 123;
> use(var);
> }
>
> then nothing prevents you from using "var" in the "if" branch, where this
> variable is not initialized. If instead you write
>
> if (xyz) {
> // ...
> } else {
> type var = 123;
> use(var);
> }
>
> this mistake fundamentally cannot happen -- the compiler prevents you from
> making it.
>
> Both of these examples are fine under C89, but the same point also holds
> within a single block. You want to limit where a certain identifier is
> valid, so you don't use it outside the valid scope.
>
> That's my 2c. The problem with the early returns is something I run into
> All. The. Time. I find it really annoying that I constantly have to rewrite
> nice linear code into a deeply nested structure just to work around this
> limitation.
>
> Nikita
>
