Makeup for if-statements?

[desktop]

I have made this:

if (parent[x] == &N::sentinel) {
/* Update root. */
parent[(*this).header] = y;
} else {
if (x == left[parent[x]]) {
left[parent[x]] = y;
} else {
right[parent[x]] = y;
}
}

but I don't really like it. Any suggestions for making it look better
and more clear?

[Martin M. Pedersen]

desktop wrote:

I have made this:

if (parent[x] == &N::sentinel) {
/* Update root. */
parent[(*this).header] = y;
} else {
if (x == left[parent[x]]) {
left[parent[x]] = y;
} else {
right[parent[x]] = y;
}
}

but I don't really like it. Any suggestions for making it look better
and more clear?

It is pretty and clear. No need to change it.

[David Harmon]

On Tue, 29 May 2007 17:28:13 +0200 in comp.lang.c++, desktop
<ff*@sss.comwrote,

>but I don't really like it. Any suggestions for making it look better
and more clear?

First step, "chain" the else-if and get rid of the extra level of
brackets.

if (parent[x] == &N::sentinel) {
parent[(*this).header] = y; // Update root.

} else if (x == left[parent[x]]) {
left[parent[x]] = y;

} else {
right[parent[x]] = y;
}

[Puppet_Sock]

On May 29, 11:28 am, desktop <[e-mail]:

I have made this:

if (parent[x] == &N::sentinel) {
/* Update root. */
parent[(*this).header] = y;
} else {
if (x == left[parent[x]]) {
left[parent[x]] = y;
} else {
right[parent[x]] = y;
}
}

but I don't really like it. Any suggestions for making it look better
and more clear?

There's nothing wrong with this. It's got the
K&R style of brackets, and my personal bias is
to line brackets up vertically. So I do it so:

if(parent[x] == &N::sentinel)
{
// if stuff here
}
else
{
// else case stuff here
}

And so on. But this is just a personal bias.
It's not something I'd suggest changing for
existing code that works.

Unless you have some mods to make, I'd suggest
leaving it alone. Working code shouldn't be
touched for "elegance" reasons. If it's doing
what it is supposed to do, and satisfying the
requirements of performance etc., don't risk
adding bugs.

Remember, every bug in your code was put there
by you.

If you have changes to make, where you go depends
on what you need to do. For example, if you were
to need to add a lot of new logic cases, you might
consider something like a table driven system.
Or if the processing for each case got much more
complicated you might consider putting in a function
to do the processing and keep the logic decisions
as simple and clear as reasonably possible. Or you
might consider that you need a state engine of some
kind, and maybe a class to handle it.
Socks

[Jim Langston]

"David Harmon" <so****@netcom.comwrote in message
news:46**************@news.west.earthlink.net...

On Tue, 29 May 2007 17:28:13 +0200 in comp.lang.c++, desktop
<ff*@sss.comwrote,

>>but I don't really like it. Any suggestions for making it look better
and more clear?

First step, "chain" the else-if and get rid of the extra level of
brackets.

if (parent[x] == &N::sentinel) {
parent[(*this).header] = y; // Update root.

} else if (x == left[parent[x]]) {
left[parent[x]] = y;

} else {
right[parent[x]] = y;
}

This is one instance I would even get rid of the backetrs entirely, although
opionions vary on this.

if (parent[x] == &N::sentinel)
parent[(*this).header] = y; // Update root.
else if (x == left[parent[x]])
left[parent[x]] = y;
else
right[parent[x]] = y;

[James Kanze]

On May 30, 9:29 am, "Jim Langston" <[e-mail]:

"David Harmon" <[e-mail] in message
news:46**************@news.west.earthlink.net...

On Tue, 29 May 2007 17:28:13 +0200 in comp.lang.c++, desktop
<[e-mail],

>but I don't really like it. Any suggestions for making it look better
and more clear?

First step, "chain" the else-if and get rid of the extra level of
brackets.

if (parent[x] == &N::sentinel) {
parent[(*this).header] = y; // Update root.
} else if (x == left[parent[x]]) {
left[parent[x]] = y;
} else {
right[parent[x]] = y;
}

This is one instance I would even get rid of the backetrs
entirely, although opionions vary on this.

if (parent[x] == &N::sentinel)
parent[(*this).header] = y; // Update root.
else if (x == left[parent[x]])
left[parent[x]] = y;
else
right[parent[x]] = y;

I think it depends somewhat on the bracketing style. Something
like:

if (parent[x] == &N::sentinel)
{
parent[(*this).header] = y; // Update root.
}
else if (x == left[parent[x]])
{
left[parent[x]] = y;
}
else
{
right[parent[x]] = y;
}

is pretty heavy. The post you were responding to, however,
treats the brackets more or less as part of the spelling of the
keywords, or at least as part of the syntax of if/else. Much
the way Modula or Ada does (except that they aren't brackets in
Modula or Ada).

My own preference is for the first style, but I'll use whatever
the company which pays me says to use. If the company style
normally puts both the opening and closing brackets on a line by
themselves, I have no compunction about dropping them if there
is only a single controlled statement. If it doesn't, and uses
something like the first example, I won't drop them, ever,
because the fact that they aren't there is far less visible.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[Victor Bazarov]

Jim Langston wrote:

"David Harmon" <so****@netcom.comwrote in message
news:46**************@news.west.earthlink.net...

>On Tue, 29 May 2007 17:28:13 +0200 in comp.lang.c++, desktop
<ff*@sss.comwrote,

>>but I don't really like it. Any suggestions for making it look
better and more clear?

First step, "chain" the else-if and get rid of the extra level of
brackets.

if (parent[x] == &N::sentinel) {
parent[(*this).header] = y; // Update root.

} else if (x == left[parent[x]]) {
left[parent[x]] = y;

} else {
right[parent[x]] = y;
}

This is one instance I would even get rid of the backetrs entirely,
although opionions vary on this.

if (parent[x] == &N::sentinel)
parent[(*this).header] = y; // Update root.
else if (x == left[parent[x]])
left[parent[x]] = y;
else
right[parent[x]] = y;

(parent[x] == &N::sentinel ?
parent[this->header]
: x == left[parent[x]] ? left[parent[x]]
: right[parent[x]] ) = y;

:-)

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

[Zeppe]

Victor Bazarov wrote:

Jim Langston wrote:

>"David Harmon" <so****@netcom.comwrote in message
news:46**************@news.west.earthlink.net.. .

>>On Tue, 29 May 2007 17:28:13 +0200 in comp.lang.c++, desktop
<ff*@sss.comwrote,
but I don't really like it. Any suggestions for making it look
better and more clear?
First step, "chain" the else-if and get rid of the extra level of
brackets.

if (parent[x] == &N::sentinel) {
parent[(*this).header] = y; // Update root.

} else if (x == left[parent[x]]) {
left[parent[x]] = y;

} else {
right[parent[x]] = y;
}

This is one instance I would even get rid of the backetrs entirely,
although opionions vary on this.

if (parent[x] == &N::sentinel)
parent[(*this).header] = y; // Update root.
else if (x == left[parent[x]])
left[parent[x]] = y;
else
right[parent[x]] = y;

(parent[x] == &N::sentinel ?
parent[this->header]
: x == left[parent[x]] ? left[parent[x]]
: right[parent[x]] ) = y;

:-)

why not
(parent[x] == &N::sentinel && (parent[(*this).header] = y, true)) || (x
== left[parent[x]] && (left[parent[x]] = y, true)) || (right[parent[x]]
= y, true);

:)

Regards,

Zeppe

[Jon Harrop]

desktop wrote:

I have made this:

if (parent[x] == &N::sentinel) {
/* Update root. */
parent[(*this).header] = y;
} else {
if (x == left[parent[x]]) {
left[parent[x]] = y;
} else {
right[parent[x]] = y;
}
}

but I don't really like it. Any suggestions for making it look better
and more clear?

This is the best you can do in C++ but I should note that you have manually
compiled a pattern match. You would get much better results (shorter,
faster code) if you wrote the pattern match in its original form and let a
compiler do the work for you, creating all of the nested ifs.

http://www.ffconsultancy.com/ocaml/b..._matching.html

--
Dr Jon D Harrop, Flying Frog Consultancy
OCaml for Scientists
http://www.ffconsultancy.com/product...ntists/?usenet

[Jim Langston]

"James Kanze" <ja*********@gmail.comwrote in message
news:11*********************@m36g2000hse.googlegro ups.com...
On May 30, 9:29 am, "Jim Langston" <[e-mail]:

"David Harmon" <[e-mail] in message
news:46**************@news.west.earthlink.net...

On Tue, 29 May 2007 17:28:13 +0200 in comp.lang.c++, desktop
<[e-mail],

>but I don't really like it. Any suggestions for making it look better
and more clear?

First step, "chain" the else-if and get rid of the extra level of
brackets.

if (parent[x] == &N::sentinel) {
parent[(*this).header] = y; // Update root.
} else if (x == left[parent[x]]) {
left[parent[x]] = y;
} else {
right[parent[x]] = y;
}

This is one instance I would even get rid of the backetrs
entirely, although opionions vary on this.

if (parent[x] == &N::sentinel)
parent[(*this).header] = y; // Update root.
else if (x == left[parent[x]])
left[parent[x]] = y;
else
right[parent[x]] = y;

I think it depends somewhat on the bracketing style. Something
like:

if (parent[x] == &N::sentinel)
{
parent[(*this).header] = y; // Update root.
}
else if (x == left[parent[x]])
{
left[parent[x]] = y;
}
else
{
right[parent[x]] = y;
}

is pretty heavy. The post you were responding to, however,
treats the brackets more or less as part of the spelling of the
keywords, or at least as part of the syntax of if/else. Much
the way Modula or Ada does (except that they aren't brackets in
Modula or Ada).

My own preference is for the first style, but I'll use whatever
the company which pays me says to use. If the company style
normally puts both the opening and closing brackets on a line by
themselves, I have no compunction about dropping them if there
is only a single controlled statement. If it doesn't, and uses
something like the first example, I won't drop them, ever,
because the fact that they aren't there is far less visible.

------------------

Ahh, you caught me. Yes, I use a bracket on a line by itself asy our
example showed, which is why I sometimes drop them for single statements.

[James Kanze]

On May 30, 2:17 pm, "Victor Bazarov" <[e-mail]:

Jim Langston wrote:

"David Harmon" <[e-mail] in message
news:46**************@news.west.earthlink.net...

On Tue, 29 May 2007 17:28:13 +0200 in comp.lang.c++, desktop
<[e-mail],
but I don't really like it. Any suggestions for making it look
better and more clear?

First step, "chain" the else-if and get rid of the extra level of
brackets.

if (parent[x] == &N::sentinel) {
parent[(*this).header] = y; // Update root.
} else if (x == left[parent[x]]) {
left[parent[x]] = y;
} else {
right[parent[x]] = y;
}

This is one instance I would even get rid of the backetrs entirely,
although opionions vary on this.

if (parent[x] == &N::sentinel)
parent[(*this).header] = y; // Update root.
else if (x == left[parent[x]])
left[parent[x]] = y;
else
right[parent[x]] = y;

(parent[x] == &N::sentinel ?
parent[this->header]
: x == left[parent[x]] ? left[parent[x]]
: right[parent[x]] ) = y;

:-)

Why the smiley? It's actually not bad, although I'd reformat
slightly:
( parent[x] == &N::sentinel
? parent[this->header]
: x == left[parent[x]]
? left[parent[x]]
: right[parent[x]] )
= y ;
The one thing that would make me hesitate is that I'm not sure
that many people are familiar with the way ?: chains (even
though it is exactly like if/else). To tell the truth, I'd
never really realized it myself until John Potter pointed it out
(and suggested that the above could be a "standard" idiom).

I also feel a little unconfortable using ?: for lvalues. But
that's probably just because I'm not used to it. But it makes
it especially clear that the goal is to assign y to something,
and that we always, unconditionally, update *something* with y.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[James Kanze]

On May 30, 3:04 pm, Zeppe
<zep_p@.remove.all.this.long.comment.yahoo.itwrote :

Victor Bazarov wrote:

Jim Langston wrote:

"David Harmon" <[e-mail] in message
news:46**************@news.west.earthlink.net.. .
On Tue, 29 May 2007 17:28:13 +0200 in comp.lang.c++, desktop
<[e-mail],
but I don't really like it. Any suggestions for making it look
better and more clear?
First step, "chain" the else-if and get rid of the extra level of
brackets.

> if (parent[x] == &N::sentinel) {
parent[(*this).header] = y; // Update root.
} else if (x == left[parent[x]]) {
left[parent[x]] = y;
} else {
right[parent[x]] = y;
}
This is one instance I would even get rid of the backetrs entirely,
although opionions vary on this.

if (parent[x] == &N::sentinel)
parent[(*this).header] = y; // Update root.
else if (x == left[parent[x]])
left[parent[x]] = y;
else
right[parent[x]] = y;

(parent[x] == &N::sentinel ?
parent[this->header]
: x == left[parent[x]] ? left[parent[x]]
: right[parent[x]] ) = y;

:-)

why not
(parent[x] == &N::sentinel && (parent[(*this).header] = y, true)) || (x
== left[parent[x]] && (left[parent[x]] = y, true)) || (right[parent[x]]
= y, true);

Because unlike Victor's example, that doesn't express the
intent. The intent is to set a particular element to y. Which
element varies, according to the structure of the hierarchy, but
that's a secondary detail. So we put it in a sub-expression of
the main expression, which is "<something= y". The key to
Victor's expression is that the "= y" is an unconditional part
of the top expression, and you don't have to check three
different branches to verify that they all end in an "= y".

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[Zeppe]

James Kanze wrote:

On May 30, 3:04 pm, Zeppe

>why not
(parent[x] == &N::sentinel && (parent[(*this).header] = y, true)) || (x
== left[parent[x]] && (left[parent[x]] = y, true)) || (right[parent[x]]
= y, true);

Because unlike Victor's example, that doesn't express the
intent. The intent is to set a particular element to y. Which
element varies, according to the structure of the hierarchy, but
that's a secondary detail. So we put it in a sub-expression of
the main expression, which is "<something= y". The key to
Victor's expression is that the "= y" is an unconditional part
of the top expression, and you don't have to check three
different branches to verify that they all end in an "= y".

That's true. It was actually a joke to point out that the Victor's
example was too tricky to understand not having any need to be so (as it
was his intent, given the smiley). I actually would leave the
conditional operator for really simple tests or for some situations that
are difficult to solve otherwise (for example, a test in a copy
constructor).

I'm not afraid of the complex instructions, just I think that they
shouldn't be used if there is no need. And, in that case, that
expression in my opinion doesn't replicate the way the human brain reasons.

Regards,

Zeppe