Hi, everyeone,
I recently stumbled on some code that someone else wrote that I don't like.
However, I'm having trouble articulating what the bad quality of the
following code is. The unnecessary use of indentation and else statements
seems to be counter-intuitive to me. However, I'm hoping for an argument
that is more formal than my intuition.
<quote>
// Make a function call based on each parameter not meeting certain
conditions.
if (a == 3)
error_code = 1;
else
{
if (b == 10)
error_code = 2;
else {
if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
}
}
return error_code;
</quote>
Personally, I find all of the else statements distracting. Is there
anything that you don't like about the organization of this simple code?
Thanks,
Scott
--
Remove .nospam from my e-mail address to mail me. http://www.e-scott.net 18  1764 
"Scott Brady Drummonds" <sc**********************@intel.com> wrote in
message news:ck**********@news01.intel.com... Hi, everyeone,
I recently stumbled on some code that someone else wrote that I don't
like. However, I'm having trouble articulating what the bad quality of the
following code is. The unnecessary use of indentation and else statements
seems to be counter-intuitive to me. However, I'm hoping for an argument
that is more formal than my intuition.
<quote>
// Make a function call based on each parameter not meeting certain
conditions.
if (a == 3)
error_code = 1;
else
{
if (b == 10)
error_code = 2;
else {
if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
}
}
return error_code;
</quote>
Personally, I find all of the else statements distracting. Is there
anything that you don't like about the organization of this simple code?
I think it's better to reduce the indentation and write
if (a == 3)
error_code = 1;
else if (b == 10)
error_code = 2;
else if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
For readability/maintainability reasons, I'd also add some braces
(if(...){}), but they aren't required.
--
David Hilsee
On Fri, 15 Oct 2004 16:05:55 -0700, Scott Brady Drummonds wrote: Hi, everyeone,
I recently stumbled on some code that someone else wrote that I don't like.
However, I'm having trouble articulating what the bad quality of the
following code is. The unnecessary use of indentation and else statements
seems to be counter-intuitive to me. However, I'm hoping for an argument
that is more formal than my intuition.
<quote>
// Make a function call based on each parameter not meeting certain
conditions.
if (a == 3)
error_code = 1;
else
{
if (b == 10)
error_code = 2;
else {
if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
}
}
return error_code;
</quote>
Personally, I find all of the else statements distracting. Is there
anything that you don't like about the organization of this simple code?
I don't see anything "wrong" with the code, but do we all understand the
programmer's intent here? If a is 3, then you don't even check any of the
others, and if a's not 3, then you check b, and so on.
I suppose he could been more concise:
if (a == 3)
error_code = 1; }
else if (b == 10)
error_code = 2;
else if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
Cheers!
Rich
Scott Brady Drummonds wrote: I recently stumbled on some code that someone else wrote that I don't
like. However, I'm having trouble articulating what the bad quality of the
following code is. The unnecessary use of indentation and else statements
seems to be counter-intuitive to me. However, I'm hoping for an argument
that is more formal than my intuition.
Thank you for displaying greater than average sensitivity to low-quality
code.
<quote>
// Make a function call based on each parameter not meeting certain
conditions.
if (a == 3)
error_code = 1;
else
{
if (b == 10)
error_code = 2;
else {
if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
}
}
return error_code;
</quote>
Personally, I find all of the else statements distracting.
If C++ supported an std::map that was slightly easier to initialize, we
could write something like this (represented in a language resembling Ruby):
map = { 3=>1, 10=>2, 7=>3 }
error_code = map.fetch(b, nil)
error_code = call_function(a,b,c) if not error_code
However, in C++ the cost of setting up such a map may exceed the cost of the
chain of else statements.
Is there
anything that you don't like about the organization of this simple code?
Why does c only get checked if a and b fail? What happens if the programmer
tries to change c's behavior? Could a, b, & c live inside a polymorphic
object? If there were some other reason to abstract them, then eventually
you might get (in C++):
error_code = abc.convertErrorCode();
Then the results vary based on the derived type of abc. If the program
containing that code depends on many similar if statements, scattered here
and there, then maybe many of them would go away with a careful use of
polymorphism.
If not, take out some excess delimiters, to help the code look like a table:
if (a == 3) error_code = 1;
else if (b == 10) error_code = 2;
else if (c == 7) error_code = 3;
else
error_code = call_function(a,b,c);
The cruft is still there, but (in a monospaced font) you can at least scan
the columns and instantly see what's going on.
--
Phlip http://industrialxp.org/community/bi...UserInterfaces
Phlip wrote: If C++ supported an std::map that was slightly easier to initialize, we
could write something like this (represented in a language resembling
Ruby):
map = { 3=>1, 10=>2, 7=>3 }
error_code = map.fetch(b, nil)
error_code = call_function(a,b,c) if not error_code
However, in C++ the cost of setting up such a map may exceed the cost of
the chain of else statements.
We would also introduce some extra stuff to fetch with a, b, or c, too!
--
Phlip http://industrialxp.org/community/bi...UserInterfaces
Scott Brady Drummonds wrote: I recently stumbled on some code that someone else wrote that I don't like.
However, I'm having trouble articulating
what the bad quality of the following code is.
The unnecessary use of indentation and else statements
seems to be counter-intuitive to me.
However, I'm hoping for an argument that is more formal than my intuition.
<quote>
// Make a function call based on each parameter not meeting certain
conditions.
if (a == 3)
error_code = 1;
else
{
if (b == 10)
error_code = 2;
else {
if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
}
}
return error_code;
</quote>
Personally, I find all of the else statements distracting.
Is there anything that you don't like
about the organization of this simple code?
cat f.cc
int call_function(int, int, int);
int f(int a, int b, int c) {
return ( 3 == a)? 1:
(10 == b)? 2:
( 7 == c)? 3: call_function(a, b, c);
}
E. Robert Tisdale wrote: > cat f.cc
int call_function(int, int, int);
int f(int a, int b, int c) {
return ( 3 == a)? 1:
(10 == b)? 2:
( 7 == c)? 3: call_function(a, b, c);
}
Another quality of good code is how easily one can change it. Even if the
stack of else statements that we recommended were just as complex as this
chain of ternary operators, changing that statement seems higher risk.
OTOH proper respects for putting the constants on the left of the ==
operator - I forgot that one.
--
Phlip http://industrialxp.org/community/bi...UserInterfaces
In article <ck**********@news01.intel.com>,
"Scott Brady Drummonds" <sc**********************@intel.com> wrote: Hi, everyeone,
I recently stumbled on some code that someone else wrote that I don't like.
However, I'm having trouble articulating what the bad quality of the
following code is. The unnecessary use of indentation and else statements
seems to be counter-intuitive to me. However, I'm hoping for an argument
that is more formal than my intuition.
<quote>
// Make a function call based on each parameter not meeting certain
conditions.
if (a == 3)
error_code = 1;
else
{
if (b == 10)
error_code = 2;
else {
if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
}
}
return error_code;
</quote>
Personally, I find all of the else statements distracting. Is there
anything that you don't like about the organization of this simple code?
It implies that the 'if (a==3)' is a more important comparison that any
of the others, when in fact they are all on the same level...
if ( a == 3 )
error_code = 1;
else if ( b == 10 )
error_code = 2;
else if ( c == 7 )
error_code = 3;
else
error_code = call_function( a, b, c );
even this implies that the comparisons are more important than assigning
to error_code, which is also (probably) incorrect...
error_code = (a==3) ? 1: (b==10) ? 2: (c==7) ? 3: call_function(a, b, c);
Of course the use of the ?: operator may upset some people, they may
even consider this obfuscation; but it succeeds in making the most
important part of the code, the most obvious. Try reading each one out
loud and you will see that the last snippet is the most coherent.
On Fri, 15 Oct 2004 16:45:24 -0700, "E. Robert Tisdale"
<E.**************@jpl.nasa.gov> wrote: > cat f.cc
int call_function(int, int, int);
int f(int a, int b, int c) {
return ( 3 == a)? 1:
(10 == b)? 2:
( 7 == c)? 3: call_function(a, b, c);
}
If indeed we do simply return afterwards, you can say:
if (a == 3) then return 1;
if (b == 10) then return 2;
if (c == 7) then return 3;
return call_function(a, b, c);
Though I usually set up an error system and write stuff like:
my_runtime_assert(a <> 3, 1);
etc.
J.
On Sat, 16 Oct 2004 00:15:07 GMT, JXStern <JX**************@gte.net>
wrote: If indeed we do simply return afterwards, you can say:
if (a == 3) then return 1;
if (b == 10) then return 2;
if (c == 7) then return 3;
return call_function(a, b, c);
Maybe without the "then" will compile better.
J.
Though I usually set up an error system and write stuff like:
my_runtime_assert(a <> 3, 1);
etc.
J.
"David Hilsee" <da*************@yahoo.com> wrote in message
news:NL********************@comcast.com... "Scott Brady Drummonds" <sc**********************@intel.com> wrote in
message news:ck**********@news01.intel.com... Hi, everyeone,
I recently stumbled on some code that someone else wrote that I don't
like. However, I'm having trouble articulating what the bad quality of the
following code is. The unnecessary use of indentation and else
statements seems to be counter-intuitive to me. However, I'm hoping for an
argument that is more formal than my intuition.
<quote>
// Make a function call based on each parameter not meeting certain
conditions.
if (a == 3)
error_code = 1;
else
{
if (b == 10)
error_code = 2;
else {
if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
}
}
return error_code;
</quote>
Personally, I find all of the else statements distracting. Is there
anything that you don't like about the organization of this simple code?
I think it's better to reduce the indentation and write
if (a == 3)
error_code = 1;
else if (b == 10)
error_code = 2;
else if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
For readability/maintainability reasons, I'd also add some braces
(if(...){}), but they aren't required.
I didn't answer your question. You said that you had a hard time
articulating what was wrong with the original code. After comparing the
improved code to the old code, it should be obvious that the flow and 4
potential outcomes are obfuscated by the excessive indentation in the old
code. Most programmers agree that indentation levels should be limited so
the function's execution paths are immediately obvious to the reader, and
many style guides contain rules that limit indentation levels for that
reason. For example, the Linux kernel style guide dictates that tabs are
eight character, and therefore code that exceeds three indentation levels
generally has to be "fixed" to make it readable. It's just harder to follow
code with many levels of indentation.
--
David Hilsee
JXStern wrote: E. Robert Tisdale wrote:
> cat f.cc
int call_function(int, int, int);
int f(int a, int b, int c) {
return ( 3 == a)? 1:
(10 == b)? 2:
( 7 == c)? 3: call_function(a, b, c);
}
If indeed we do simply return afterwards, you can say:
if (a == 3) then return 1;
if (b == 10) then return 2;
if (c == 7) then return 3;
return call_function(a, b, c);
No. They won't let you do that here.
The fatwa declares that
functions shall have a single point of return.
But that's a 'nother can of worms.
In article <ck**********@nntp1.jpl.nasa.gov>,
E. Robert Tisdale <E.**************@jpl.nasa.gov> wrote: No. They won't let you do that here.
The fatwa declares that
functions shall have a single point of return.
But that's a 'nother can of worms.
So exceptions aren't allowed?
--
Mark Ping em****@soda.CSUA.Berkeley.EDU
E. Mark Ping wrote: E. Robert Tisdale wrote:
No. They won't let you do that here.
The fatwa declares that
functions shall have a single point of return.
But that's a 'nother can of worms.
So exceptions aren't allowed?
The higher rule is "functions shall be short".
If you indulge in long ones, you need extra rules to help you mentally sort
out their concepts.
"Single exit" might have other reasons...
--
Phlip http://industrialxp.org/community/bi...UserInterfaces
* Scott Brady Drummonds:
I recently stumbled on some code that someone else wrote that I don't like.
However, I'm having trouble articulating what the bad quality of the
following code is. The unnecessary use of indentation and else statements
seems to be counter-intuitive to me. However, I'm hoping for an argument
that is more formal than my intuition.
<quote>
// Make a function call based on each parameter not meeting certain
conditions.
if (a == 3)
error_code = 1;
else
{
if (b == 10)
error_code = 2;
else {
if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
}
}
return error_code;
</quote>
Personally, I find all of the else statements distracting. Is there
anything that you don't like about the organization of this simple code?
1) Indentation that shouldn't be there.
2) Inconsistent placement of braces.
3) Lacking braces around if-part and else-part.
4) Superflous variable (assuming error_code isn't global which would be
even worse).
5) Single-letter variable names.
6) "Magic numbers" (literal constants) sprinkled like pepper.
7) Error code instead of exceptions and asserts.
The 'else's are OK and should be there, as far as I'm concerned. They
prevent a common problem with maintainance where new statements are
inserted between if's. We want them to have to work harder to screw up,
to get compilation errors for the most common screw-up modes.
Regarding 1, 2, 3 and 4, try (adjust to your favorite formatting)
if ( a == 3 ) { return 1; }
else if( b == 10 ) { return 2; }
else if( c == 7 ) { return 3; }
else { return call_function( a, b, c ); }
or
return (
a == 3? 1 :
b == 10? 2 :
c == 7? 3 :
call_function( a, b, c )
);
but keep in mind that 7, exceptions, can make things simpler, e.g.
throwIf( a == 3 ); throwIf( b == 10 ); throwIf( c == 7 );
call_function( a, b, c );
--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?
"JXStern" <JX**************@gte.net> skrev i melding
news:c5********************************@4ax.com... On Sat, 16 Oct 2004 00:15:07 GMT, JXStern <JX**************@gte.net>
wrote:If indeed we do simply return afterwards, you can say:
if (a == 3) then return 1;
if (b == 10) then return 2;
if (c == 7) then return 3;
return call_function(a, b, c);
Maybe without the "then" will compile better.
Previous Visual Basic coder ey? :) that is at least the only language I know
of that use " if then".
- Magnus
On Fri, 15 Oct 2004 23:28:58 UTC, "Phlip" <ph*******@yahoo.com> wrote: Scott Brady Drummonds wrote:
I recently stumbled on some code that someone else wrote that I don't
like. However, I'm having trouble articulating what the bad quality of the
following code is. The unnecessary use of indentation and else statements
seems to be counter-intuitive to me. However, I'm hoping for an argument
that is more formal than my intuition.
Thank you for displaying greater than average sensitivity to low-quality
code.
<quote>
// Make a function call based on each parameter not meeting certain
conditions.
if (a == 3)
error_code = 1;
else
{
if (b == 10)
error_code = 2;
else {
if (c == 7)
error_code = 3;
else
error_code = call_function(a,b,c);
}
}
return error_code;
</quote>
Personally, I find all of the else statements distracting.
If C++ supported an std::map that was slightly easier to initialize, we
could write something like this (represented in a language resembling Ruby):
map = { 3=>1, 10=>2, 7=>3 }
error_code = map.fetch(b, nil)
error_code = call_function(a,b,c) if not error_code
However, in C++ the cost of setting up such a map may exceed the cost of the
chain of else statements.
Is there
anything that you don't like about the organization of this simple code?
Why does c only get checked if a and b fail? What happens if the programmer
tries to change c's behavior? Could a, b, & c live inside a polymorphic
object? If there were some other reason to abstract them, then eventually
you might get (in C++):
error_code = abc.convertErrorCode();
Then the results vary based on the derived type of abc. If the program
containing that code depends on many similar if statements, scattered here
and there, then maybe many of them would go away with a careful use of
polymorphism.
If not, take out some excess delimiters, to help the code look like a table:
if (a == 3) error_code = 1;
else if (b == 10) error_code = 2;
else if (c == 7) error_code = 3;
else
error_code = call_function(a,b,c);
The cruft is still there, but (in a monospaced font) you can at least scan
the columns and instantly see what's going on.
When all I need are a few similar statements that aren't multi-line
that would be very close to my code as well. I'd also have the last
line line up as best as possible...
....
else if (c == 7) error_code = 3;
else error_code = call_function (a, b, c);
....
I prefer this kind of style since it is very easy to eye-ball
it and see what should and shouldn't be there. It can also be
easy to add a line and be pretty sure you got it right. For
example, a misspelling of "error_xode" would stick out like a
sore thumb (whatever that looks like).
As the code gets more complex I rewrite it as necessary
back in the traditional style. My emphasis is on speed
reading, understandability, and maintainability.
The original code fragment is rather difficult to read
and comprehend quickly when scrolling or paging through the
code. It also has inconsistant use of several constructs.
Our team has a good deal of legacy code and we have a
lot of styles to deal with. So long as we can be reasonably
sure we're making equivalent changes when correcting style
we normalize code in close proximity to other changes as
time permits. This allows the code to become more consistant
and for coding standards to evolve over time.
David (and his $0.02 USD worth)
David wrote: When all I need are a few similar statements that aren't multi-line
that would be very close to my code as well. I'd also have the last
line line up as best as possible...
...
else if (c == 7) error_code = 3;
else error_code = call_function (a, b, c);
...
There are those whose fetish for automated reformatting tools is so strong
they will run it on that, blindly, and screw up the scanning.
I prefer this kind of style since it is very easy to eye-ball
it and see what should and shouldn't be there. It can also be
easy to add a line and be pretty sure you got it right. For
example, a misspelling of "error_xode" would stick out like a
sore thumb (whatever that looks like).
Ideally, designs should fold duplicated statements together. When you can't
think of a way to merge such statements without causing obfuscation (or when
you can't think of any way at all), you should at least put duplicated
statements close to each other. This practice forms little tables that are
relatively easy to scan and extend.
However, the primary system to ensure code can change easily is unit tests.
See below.
As the code gets more complex I rewrite it as necessary
back in the traditional style. My emphasis is on speed
reading, understandability, and maintainability.
You imply that the "traditional" style does not emphasize reading,
understanding, or mantainability. ;-)
The original code fragment is rather difficult to read
and comprehend quickly when scrolling or paging through the
code. It also has inconsistant use of several constructs.
Our team has a good deal of legacy code and we have a
lot of styles to deal with. So long as we can be reasonably
sure we're making equivalent changes when correcting style
we normalize code in close proximity to other changes as
time permits. This allows the code to become more consistant
and for coding standards to evolve over time.
You are being too dainty.
Firstly, if you read and understand a stretch of code, such as during bug
removal, it's good to leverage that new understanding by cleaning up the
code, while it is fresh in your mind, and you have another reason to change
it.
However, I sense that your project has no unit tests, and that your team
thinks adding them will slow you down. Read the new book /Working
Effectively with Legacy Code/ by Mike Feathers (then /Refactoring to
Patterns/ by Josh Kerievsky). Those books make an excellent case for adding
unit tests to testless code. Specifically, they _define_ "legacy" code as
code without unit tests - code written the "traditional" way.
If you read and understand a stretch of code, such as during bug removal,
it's good to leverage that new understanding by writing a unit test which
captures the bug before killing it. Then run all such tests every minute or
so for the rest of the project. This is a much safer strategy than making
only "safe" changes.
--
Phlip http://industrialxp.org/community/bi...UserInterfaces
On Sat, 16 Oct 2004 22:10:33 +0200, "Magnus" <no@spam.com> wrote: "JXStern" <JX**************@gte.net> skrev i melding
news:c5********************************@4ax.com.. . On Sat, 16 Oct 2004 00:15:07 GMT, JXStern <JX**************@gte.net>
wrote: >If indeed we do simply return afterwards, you can say:
>
> if (a == 3) then return 1;
> if (b == 10) then return 2;
> if (c == 7) then return 3;
>
> return call_function(a, b, c);
Maybe without the "then" will compile better.
Previous Visual Basic coder ey? :) that is at least the only language I know
of that use " if then".
Previous Algol W coder, and maybe fifty others, of which about
forty-five use "then" (Lisp doesn't, nor Fortran II, nor raw
assembler, ...) But I think I always say it mentally in any case.
Mea culpa.
J. This thread has been closed and replies have been disabled. Please start a new discussion. Similar topics
by: Harold Crump |
last post by:
Greetings,
I have a PHP/MySQL application that I am deploying at a client's.
I am fairly certain that they will steal my source code and re-sell to
other companies.
I would like to somehow...
|
by: Rasmus Fogh |
last post by:
Dear All,
I need a way of writing strings or arbitrary Python code that will
a) allow the strings to be read again unchanged (like repr)
b) write multiline strings as multiline strings instead...
|
by: Rasmus Fogh |
last post by:
Someone raised the question of automatic code generation a few weeks back.
And yes, we (CCPN) are using automatic Python code generation in a major
way. Basically we are making data models in...
|
by: Irmen de Jong |
last post by:
What would be the best way, if any, to obtain
the bytecode for a given loaded module?
I can get the source:
import inspect
import os
src = inspect.getsource(os)
but there is no...
|
by: Sky Fly |
last post by:
Hi,
I know that when an .NET exe is run, the CLR loads
the exe (along with dependent assemblies),
compiles them to native code then runs the code.
Assuming the assemblies are loaded from a...
|
by: James Cameron |
last post by:
Hi I'm developing a program and the client is worried about future
reuse of the code. Say 5, 10, 15 years down the road. This will be a
major factor in selecting the development language. Any...
|
by: DPfan |
last post by:
What's exactly the meaning of "code reuse" in C++?
Why such kind of reuse have more advantages over the counterpart in other
language like in C?
How is "code reuse" realized in C++? By...
|
by: geek04 |
last post by:
i'm using pro*c to precompile my c++ code which accesses oracle 9i
database, i'm running a oracle 9i client on my system
on compiling the c++ file (generated by pro*c)i'm getting following...
|
by: ED |
last post by:
I currently have vba code that ranks employees based on their average
job time ordered by their region, zone, and job code. I currently
have vba code that will cycle through a query and ranks each...
|
by: Fuzzyman |
last post by:
Hello all,
I'm trying to extract the code object from a function, and exec it
without explicitly passing parameters.
The code object 'knows' it expects to receive paramaters. It's
'arg_count'...
|
by: Charles Arthur |
last post by:
How do i turn on java script on a villaon, callus and itel keypad mobile phone
|
by: ryjfgjl |
last post by:
In our work, we often receive Excel tables with data in the same format. If we want to analyze these data, it can be difficult to analyze them because the data is spread across multiple Excel files...
|
by: emmanuelkatto |
last post by:
Hi All, I am Emmanuel katto from Uganda. I want to ask what challenges you've faced while migrating a website to cloud.
Please let me know.
Thanks!
Emmanuel
|
by: BarryA |
last post by:
What are the essential steps and strategies outlined in the Data Structures and Algorithms (DSA) roadmap for aspiring data scientists? How can individuals effectively utilize this roadmap to progress...
|
by: nemocccc |
last post by:
hello, everyone, I want to develop a software for my android phone for daily needs, any suggestions?
|
by: Sonnysonu |
last post by:
This is the data of csv file
1 2 3
1 2 3
1 2 3
1 2 3
2 3
2 3
3
the lengths should be different i have to store the data by column-wise with in the specific length.
suppose the i have to...
|
by: Hystou |
last post by:
There are some requirements for setting up RAID:
1. The motherboard and BIOS support RAID configuration.
2. The motherboard has 2 or more available SATA protocol SSD/HDD slots (including MSATA, M.2...
|
by: marktang |
last post by:
ONU (Optical Network Unit) is one of the key components for providing high-speed Internet services. Its primary function is to act as an endpoint device located at the user's premises. However,...
|
by: Hystou |
last post by:
Most computers default to English, but sometimes we require a different language, especially when relocating. Forgot to request a specific language before your computer shipped? No problem! You can...
| |
