if (!dry_run && ((stdout_closed = true), close_stream (stdout) != 0))
The comma operator is a fundamental
operator in C that doesn't get nearly the exposure that other, more often used
operators such as '=' or '++' get. Its purpose is to join together expressions into
one line in a very specific way: the expression to the left of the comma
is first evaluated and the result is ignored, then the expression to the
right is evaluated and the result is returned.
The Wikipedia article has a few rather good use cases for it that
you should read. The upshot is that it can be helpful in grouping expressions
together in code to improve the readability of the code. In my opinion this is
a slight advantage that introduces its own disadvantages. I'll illustrate with
an example.
While I don't remember when I first saw
the comma operator for the first time, it came back into the forefront of my
mind when I was working on writing a circular buffer library for an upcoming
article. Circular buffers involve reading and writing lots of memory which
means lots of loops. A typical memory copy (for a linear buffer) looks
something like this:
void memcpy(void * dest, void * src, size_t len)
{
int i;
for(i=0;i<len;i++)
dest[i] = src[i];
}
This function relies on the fact that the
source buffer is linear. The next memory address is always right after the
current one : all you need to do to get there is increment. A circular buffer
doesn't work the same way because it's topology is (obviously) a circle: the
source pointer can start off anywhere within the buffer and will wrap back to
the start of the buffer when it reaches the end. This makes it more complicated
to write a memory copy out of a circular buffer - it might look something like
this:
void cbRead(cbRef buffer, void * dest, size_t len)
{
int i;
for(i=0;i<len;i++)
{
dest[i] = buffer.data[buffer.srcPtr++]
if(buffer.srcPtr == buffer.length-1)
buffer.srcPtr = 0;
}
}
(Note that this code
isn't complete by any means - at the very least there's no check to ensure
there's enough data in the buffer).
In the circular buffer
case, the buffer maintains a source pointer that indicates what element inside
of its data array has the first byte of data. This pointer has to be wrapped
back around to the beginning of the buffer by the if statement
to respect the circular nature of the buffer.
This
implementation looks clunkier than the linear buffer memory
copy and with that conditional inside the loop, I'd guess it's slower as well.
You can replace the conditional statement with something much quicker if you
are willing to live by the following restrictions:
1.
Every buffer has to be
the same, fixed size
2.
That size is a power of
2 minus one (example: 31 is 2^5 -1)
These restrictions allow
you to use bit masking to replace the if statement:
void cbRead(cbRef buffer, void * dest, size_t len)
{
int i;
for(i=0;i<len;i++)
{
dest[i] = buffer.data[buffer.srcPtr++]
buffer.srcPtr &=0x1F; //buffer
size of 31
}
}
Replacing the if statement
with a bit mask is much faster - especially on most microcontrollers, but it
still looks less sleek than the original memory copy, doesn't it? This
is where I thought the comma operator could "help" - by
allowing me to write the same functionality like this:
void cbCopy(cbRef buffer, void * dest, size_t len)
{
int i=0,endPtr;
for(endPtr=(buffer.srcPtr+len)&0x1F;srcPtr!=endPtr;buffer.srcPtr=buffer.srcPtr++,buffer.srcPtr&0x1F)
{
dest[i++] = buffer.data[buffer.srcPtr]
}
}
If you squint your eyes
a bit and ignore most things that are happening in the for loop,
this does indeed look sleek: there's only a
single line inside the for loop and it looks like a
memory copy. What's happening in the rest of it?
- I changed the for loop from one that loops on i from 0 to len to one that calculates an endPtr and then loops until the srcPtr is equal to it. This allows me to rewrite the for loop:
- The initializer generates the endPtr.
- The condition for terminating the loop is that the srcPtr is equal to the endPtr - i.e., we've copied all of the data we were asked to.
The increment is where
the magic is. It (theoretically) uses the comma operator to join together
two expressions: the first to increment the source pointer and the second
to perform a bitmask on the pointer to wrap it back to 0 when it exceeds the
length of the buffer. The comma operator should discard the result of
the first increment and then return the value of the source pointer after
it's been masked.
I love being clever as
much as the next guy and while I can admire clever, sleek pieces
of code this just isn't one them. I can't recommend that you write code this
way solely for the sake of using the comma operator for two three reasons:
1.
The comma operator is
simply much less readable than the alternatives. Most people will be
immediately confused by it and will have to fire up Google to see what's going
on. This distraction makes it harder to read your code.
2.
It's rarely (if ever)
necessary. There's always some combination of other, more readable statements
that will give you the same effect.
3.
This code won't even
work the way I expected it to: due to operator precedence, the 'b++' is evaluated
first and the value returned, and the result of the mask is ignored.
My 'inventive' and
'clever' use of the comma operator changed a fairly straightforwad loop into a
confusing mess that doesn't even fit into this article (on my browser anyhow) and
wouldn't work if I ever coded it that way. Luckily, I took one look at that
monstrosity and decided it wasn't worth it. Judging from a number of
comments I've seen while researching this article, that seems to be the general
consensus agreed to by most people (including the drafters of the MISRA C
standard, which simply forbids the use of the operator).
Still, I don't feel
right having that be the final word. Yes, in this circumstance
(and very likely, in most circumstances) using the comma operator produced code
that was less readable than the alternative. That doesn't mean it's useless -
it only means that we haven't found the appropriate use for it yet. In fact,
you'll find a few practical, non-academic examples where the comma operator
produces cleaner, more readable code. The answers to thisStackoverflow
question have a few good examples:
So don't let the general
opinion against the comma operator sway you. As with most things,
programming skill improves when the programmer is challenged - regardless of
whether the programmer passes the challenge or not! In this
case, the challenge was to rewrite a memory copy loop using the comma operator
(and maintain readability). I'd argue I failed this particular challenge but in
the process improved my skills. So I encourage you to challenge
yourself: use the comma operator somewhere - anywhere! If it works out, great!
If not, you'l have learned something - also great!
UPDATE
Originally, I created an
imaginary example using the comma operator:
a=b++,b>>2;
With the assumption that 'b++' would be
executed and the result ignored and the outcome of 'b>>2' would be
assigned to 'a' I thought the equivalent code to the above would be this:
b++;
a=b>>2;
This turned out not to be the case, but
thankfully Old Wolf corrected me here and on reddit. Due to operator precedence
rules in C he claimed, the equivalent code would actually be this:
a = b++;
b >> 2;
This would not have the
intended effect at all.
Now, I immediately
believed Old Wolf because he said I was wrong - if there's anyone I
don't trust it's me. However, sometimes I am in fact wrong about being wrong,
so to determine which kind of wrong I was this time, I wrote this program
(comma.c) to test the matter:
#include <stdio.h>
int main(void)
{
int a=0;
int b=7;
a=b++,b>>2;
printf("a is
%d\r\n",a);
printf("b is
%d\r\n",b);
return 0;
}
If I were correct I
would expect a to be 2 and b to be 8. If Old Wolf were right, a will
be 7 and b would be 8. So, which is it?
C:\Users\Stephen\Documents>gcc
comma.c
C:\Users\Stephen\Documents>a.exe
a is 7
b is 8
I knew I should never
have trusted me. My hat's off to Old Wolf. Apparently, an Old Wolf can teach a
dog like me new tricks!
It's also worth noting
that because of this error on my part, my circular buffer code wouldn't work
correctly either. Luckily, that abomination is mainly meant to show that if you
try to be too clever when writing code you'll end up making a mess of
everything. Given that I tried to be clever and ended up making a mess out of
everything, I think this article stands as sufficient proof by demonstration
that you should stick with simplicity and clarity over clever tricks.
Sign up here with your email
ConversionConversion EmoticonEmoticon