On Sat, 2022-12-03 at 22:35 +0100, Daniel Glöckner wrote:
>
> On Sat, Dec 03, 2022 at 04:20:30PM +0100, fenug...@mail.com wrote:
> > > Sent: Saturday, December 03, 2022 at 10:29 AM
> > > From: "Rene Kita" <m...@rkta.de>
> > > > 1- avoid bundling an assignment in a conditional, `if ((var = func()) >
> > > > other_var) ....`
> > >
> > > Indicating that it indeed is meant to be an assignment is usually done
> > > by wrapping it in parentheses.
> >
> > That looks to me like a "workaround" - I really see no advantage to
> > cramming all that code on one line. Terseness sacrifices readability in
> > this case IMO.
>
> How would you write
>
> while ((entry = readdir(dir)) != NULL) {
> something using entry;
> }
>
> or
>
> while ((c = fgetc(f)) != EOF) {
> something using c;
> }
>
> in a more readable way without using that construct?
You might have missed the _recommendation_ character of the
"_avoid_ mixing assignments and control flow" phrase. It's not a
hard and fast rule to _never_ do an assignment and a test for its
value within the same statement. But a strong recommendation that
saving a few key strokes is not important enough a reason to
force every reader to spare conscious resources to parse the
resulting phrase of code, _while_ they are trying to concentrate
on the application level and try to hunt down other issues. It's
about cognitive load during review and during maintenance.
I believe the most recent motivation to add that recommendation
to HACKING was bolier plate in device drivers' receive callbacks,
that reads like this:
if (!(sdi = cb_data))
return TRUE;
if (!(devc = sdi->priv))
return TRUE;
if (revents == G_IO_IN) {
/* TODO */
}
Which was considered somewhat acceptable so far, and can be found
in several places in existing code. Probably was even generated
by tools in that form. You could consider these trivial
incarnations. Still I don't like them, and would try to avoid
them. They are easy to phrase in ways that better reflect what's
happening. Are easier to verify for completeness and correctness.
Check VCS history how often existing code keeps missing the
occassional assignment in stuff that is considered boiler plate
and isn't looked at while you are staring at another location.
And notice that these assignments and checks are in a linear
sequence. Not in the condition of a loop instruction. Which
_could_ also be phrased differently but then might be as
unfortunate to read, as you state. But it's not what motivated
the HACKING paragraph, and neither was it forbidden to use it
where needed, just discouraged (strongly though).
I think we all agree on the fact that sometimes unfortunate
phrases need to be used, or can be considered acceptable compared
to their alternatives. Your loop examples all are iterators,
there are other read loops where the evaluation of the condition
is more complex (like checking for read failure, short reads,
reads of correct length yet unexpected content, etc etc) where
the simple check should also go into the body and not the loop
condition. It's always a judgement call. In every case.
The general idea is to write code for human readers, and not
worry too much about machines. It should not require a degree and
decades in C language use to see what's happening while you read
the code. Compilers may or may not combine several C language
instructions into the same machine instruction when they compile
the text to blobs. But it's humans who need to read that text a
lot, often during a task which is complex and binds essential
resources (like attention that cannot be shared, short term
memory for other events and data), and human readers should not
have to _additionally_ and _unnecessarily_ need to spare the
resources that they don't have at this time to parsing language
details that they should not have to worry about.
That's BTW also why I so disagree with the "if (!strcmp())"
phrase. Regardless of popularity, and regardless of the
theoretical possibility to "get it" after some thinking, it's
still a stumbling block every time that I come across such an
instance. Then I go "what the? if it doesn't compare then act as
if there had been a match? what the?" -- This type of distraction
just is not necessary, and is counter productive. Code is rarely
written but often gets read. Do the math which part of that is
expensive, and where saving really matters.
Existing mainline code also has other instances of mixing
function calls and variable assignments and control flow all
within a single statement. They are quite popular because saving
a few key strokes is rather important to some authors. Yet these
phrases having slipped in the past doesn't mean that they are
considered nice to have or acceptable for new submissions.
Here is an example, not the worst, not specifically saught for,
just what came up in a quick glance, but good for illustration.
Tell quickly where the function call's arguments list is, what
the routine does, which execution result is expected or checked,
what the resulting code block of the success or failure check
would be. What the outcome of that code block is when you enter
at the top and leave at the bottom.
Answer this quickly and correctly while you are juggling other
details of the driver in your head while you are looking for some
totally different issue at a different abstraction level and only
by coincidence came across that phrase while you followed complex
code paths. Yes it is legal C language source code. Yes it can
get parsed with a little time and thinking. But is it really a
good idea to add this workload to humans who don't have the
resources to spare?
usb = sdi->conn;
if ((ret = libusb_control_transfer(
usb->devhdl, LIBUSB_REQUEST_TYPE_VENDOR |
LIBUSB_ENDPOINT_OUT,
bRequest, wValue, wIndex, (unsigned char*)data, wLength,
DEFAULT_TIMEOUT_MS)) != wLength) {
sr_err("failed to write %d bytes via ctrl-out %d %#x, %d: %s.",
wLength, bRequest, wValue, wIndex,
libusb_error_name(ret));
return SR_ERR;
}
return SR_OK;
That's also why I dislike assignments in variable declarations. I
keep missing them, and it's additional work to have to scan many
more lines of text just to see -- if you can -- the initial
assignment to, subsequent updates of, and the eventual check of a
variable's value. Just too damn expensive to parse. And all of
this just to save a few key strokes? No. Not desirable.
There is more to this style guide than mere cosmetics or personal
preference on alternatives that all are equal. The preferences
that are documented are reasoned, not arbitrary. Usually. Often.
Projects may differ, as do humans. This one is sigrok. And what
the currently active developers consider important.
virtually yours
Gerhard Sittig
--
If you don't understand or are scared by any of the above
ask your parents or an adult to help you.
_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel
