Comma operator = broken design

[Regan Heath]

On Fri, 09 Dec 2011 11:39:55 -0000, Timon Gehr <timon.gehr at gmx.ch> wrote:
> These are the occurences of the comma operator in directory 'std':

Here is my /very/ quick re-write of each without looking at the context of  
each snippet.  There may be much better re-writes in context.  Re-writing  
this was problematic /because/ the comma operator makes things that much  
more confusing to me.  Especially example #5 below where the last ,  
actually separates enforce parameters!

1)

> return r2.empty ? (r1 = r, true) : false;

if (!r2.empty) return false;
r1 = r;
return true;


2)

> return binaryFun!pred(r.front, e) ? (r.popFront(), true) : false;

if (!binaryFun!pred(r.front, e)) return false;	
r.popFront();
return true;


3)

> if (f.flPlus)
>      signChar = '+', ++minw;
> else if (f.flSpace)
>      signChar = ' ', ++minw;

This is purely {}; avoidance it seems..

if (f.flPlus)
{
   signChar = '+';
   ++minw;
}
else if (f.flSpace)
{
   signChar = ' ';
   ++minw;
}


4)

> if (std.ascii.toLower(p.front) == 'n' &&
>         (p.popFront(), std.ascii.toLower(p.front) == 'f') &&
>         (p.popFront(), p.empty))

'if' statements with side effects are yuck.  I prefer the check for error  
and bail style but you could use multiple layers of 'if' instead..

if (std.ascii.toLower(p.front) != 'n') //error handling
p.popFront();
if (std.ascii.toLower(p.front) != 'f') //error handling
p.popFront();
if (!p.empty) //error handling


5)

> enforce((p.popFront(), !p.empty && std.ascii.toUpper(p.front) == 'A')
>          && (p.popFront(), !p.empty && std.ascii.toUpper(p.front) ==   
> 'N'),
>         new ConvException("error converting input to floating point"));

This is blatant enforce abuse IMO..

p.popFront();
if(p.empty || std.ascii.toUpper(p.front) != 'A'))
   throw new ConvException("error converting input to floating point"));
p.popFront();
if(p.empty || std.ascii.toUpper(p.front) != 'N'))
   throw new ConvException("error converting input to floating point"));


6)

> if (indexStart != 0)
>      formatValue(w, indexStart, f), put(w, '$');

More {}; avoidance..

if (indexStart != 0)
{
     formatValue(w, indexStart, f);
	put(w, '$');
}


7)

> if (c == '\"' || c == '\\')
>      put(w, '\\'), put(w, c);
> else
>      put(w, c);

More {}; avoidance..

if (c == '\"' || c == '\\')
{
   put(w, '\\');
   put(w, c);
}
else
{
   put(w, c);
}


8)

> return (++mi.m_cRefs, cast(HXModule)mi);

less (), one more ; and <enter>..

++mi.m_cRefs;
return cast(HXModule)mi;


9)

> return (++mi.m_cRefs, hModule);

as above..

++mi.m_cRefs;
return hModule;


10)

> return
>      c <= 0x7F ? 1
>      : c <= 0x7FF ? 2
>      : c <= 0xFFFF ? 3
>      : c <= 0x10FFFF ? 4
>      : (assert(false), 6);

*Much* clearer with the rewrite..

assert(c <= 0x10FFFF);
return
     c <= 0x7F ? 1
     : c <= 0x7FF ? 2
     : c <= 0xFFFF ? 3
     : c <= 0x10FFFF ? 4
     : 6;

None of the above look significantly harder without the comma operator and  
quite a few are far clearer (to me) without it.

R

-- 
Using Opera's revolutionary email client: http://www.opera.com/mail/