stylistic question


B

Bill Cunningham

I thought I would submit this function I wrote to the group for opinions
on style. The function itself seems to work. I know sometimes not returning
anything can lead to undefined behavior but with main returning int I
believe int is considered by the standard as the default type. So the return
0 in main isn't necessary but the returns in t_range are. t_range means true
range between security prices. Any opinions on how I can improve this
function as far as style?

#include <stdio.h>

double t_range(double hi, double low, double pc)
{
double ans1, ans2, ans3;
ans1 = ans2 = ans3 = 0.0;
ans1 = hi - low;
ans2 = pc - low;
ans3 = hi - pc;
if (ans1 > ans2 && ans1 > ans3)
return ans1;
else if (ans2 > ans3 && ans2 > ans1)
return ans2;
else if (ans3 > ans2 && ans3 > ans1)
return ans3;
else {
return -1;
}
}

int main(void)
{

double r;
r = t_range(1.25, 1.37, .67);
printf("%.2f\n", r);
return 0;
}
http://www.incrediblecharts.com/indicators/true_range.php

B
 
Ad

Advertisements

S

Stephen Sprunk

I thought I would submit this function I wrote to the group for
opinions on style. The function itself seems to work. I know
sometimes not returning anything can lead to undefined behavior but
with main returning int I believe int is considered by the standard
as the default type. So the return 0 in main isn't necessary but the
returns in t_range are.

main() is a special case in the Standard; not explicitly returning is
allowed. For any other function, you're required to return _some_ value
unless the return type is void, in which case you're required to _not_
return a value.
t_range means true range between security prices. Any opinions on how
I can improve this function as far as style?

#include <stdio.h>

double t_range(double hi, double low, double pc)
{
double ans1, ans2, ans3;
ans1 = ans2 = ans3 = 0.0;
ans1 = hi - low;
ans2 = pc - low;
ans3 = hi - pc;
if (ans1 > ans2 && ans1 > ans3)
return ans1;
else if (ans2 > ans3 && ans2 > ans1)
return ans2;
else if (ans3 > ans2 && ans3 > ans1)
return ans3;
else {
return -1;
}
}

...
http://www.incrediblecharts.com/indicators/true_range.php

That's a decent literal translation of the requirements, except the
"else"s aren't necessary since the "then" path doesn't fall through, and
your temporary variable names aren't very informative. I also wonder if
it's possible to reach the "return -1;" path at all.

However, transforming the requirements slightly can give the correct
result with significantly less complexity and code:

double t_range(double hi, double low, double pc)
{
if (hi < pc)
hi = pc;
if (low > pc)
low = pc;
return hi - low;
}

or, for those who value brevity over clarity:

double t_range(double hi, double low, double pc)
{
return (pc > hi ? pc : hi) - (pc < low ? pc : low);
}


S
 
I

Ike Naar

That's a decent literal translation of the requirements, except the
"else"s aren't necessary since the "then" path doesn't fall through, and
your temporary variable names aren't very informative. I also wonder if
it's possible to reach the "return -1;" path at all.

When the two largest ans* values are equal.
 
B

Barry Schwarz

I thought I would submit this function I wrote to the group for opinions
on style. The function itself seems to work. I know sometimes not returning
anything can lead to undefined behavior but with main returning int I
believe int is considered by the standard as the default type. So the return

The current standard does not have default types for functions. The
return type for main must be specified as int.
0 in main isn't necessary but the returns in t_range are. t_range means true

While reaching the final } in main returns a value of 0, many consider
an explicit return statement to be the preferred style.
range between security prices. Any opinions on how I can improve this
function as far as style?

Make sure the program executes correctly before worrying about style.
#include <stdio.h>

double t_range(double hi, double low, double pc)
{
double ans1, ans2, ans3;
ans1 = ans2 = ans3 = 0.0;

Delete this statement since all three variables have new values
assigned in the next three statements.
ans1 = hi - low;
ans2 = pc - low;
ans3 = hi - pc;
if (ans1 > ans2 && ans1 > ans3)
return ans1;
else if (ans2 > ans3 && ans2 > ans1)
return ans2;
else if (ans3 > ans2 && ans3 > ans1)
return ans3;
else {
return -1;

What should the result be when pc happens to equal low or hi. Consider
hi = 5, low = 3, and pc = 5. The range is 2 but you return -1.

You also return -1 when all three are equal even though 0 seems to be
a possible range, especially if the stock is not being traded.

Without doing any detailed checking, I think replacing the > with >=
might solve these problems.
}
}

int main(void)
{

double r;
r = t_range(1.25, 1.37, .67);

Generate better test cases. You should have caught the problem with
equality during unit test.
 
S

Stephen Sprunk

When the two largest ans* values are equal.

True, the above code fails to meet the specification (by returning -1)
if pc==hi and/or pc==low. Good catch.

S
 
B

Ben Bacarisse

Stephen Sprunk said:
or, for those who value brevity over clarity:

double t_range(double hi, double low, double pc)
{
return (pc > hi ? pc : hi) - (pc < low ? pc : low);
}

I don't think you need to compromise:

return fmax(hi, pc) - fmin(low, pc);

(#include <math.h> of course).
 
Ad

Advertisements

K

Keith Thompson

Bill Cunningham said:
I thought I would submit this function I wrote to the group for opinions
on style. The function itself seems to work. I know sometimes not returning
anything can lead to undefined behavior but with main returning int I
believe int is considered by the standard as the default type.

There is no "default type". Prior to C99, declaring or defining a
function with no explicit type was equivalent to declaring or defining
it with a return type of int, and calling a function with no visible
declaration would cause the compiler to assume a return type of int.
Both rules have been removed from the language.

But this is irrelevant to the code you show below. Your t_range
function is defined to return a result of type double, and all possible
execution paths cause it to return a result of type double. That
includes the "return -1;"; the int value -1 is converted to double.
IMHO "return -1.0;" would be clearer.
So the return
0 in main isn't necessary but the returns in t_range are. t_range means true
range between security prices. Any opinions on how I can improve this
function as far as style?

#include <stdio.h>

double t_range(double hi, double low, double pc)

IMHO ordering the first to parameters as "lo, hi" would be less
confusing than "hi, lo".
{
double ans1, ans2, ans3;
ans1 = ans2 = ans3 = 0.0;

The above assignment is unnecessary, since you immediately assign values
to all three variables.
ans1 = hi - low;
ans2 = pc - low;
ans3 = hi - pc;

And these assignments would be clearer as initializations:

const double ans1 = hi - low;
const double ans2 = pc - low;
const double ans3 = hi - pc;

The "const" emphasizes (and enforces) the fact that none of these
variables are ever modified after their initializations.
if (ans1 > ans2 && ans1 > ans3)
return ans1;
else if (ans2 > ans3 && ans2 > ans1)
return ans2;
else if (ans3 > ans2 && ans3 > ans1)
return ans3;
else {
return -1;
}
}

int main(void)
{

double r;
r = t_range(1.25, 1.37, .67);
printf("%.2f\n", r);
return 0;
}
http://www.incrediblecharts.com/indicators/true_range.php

I'm not commenting on the algorithm; I see others have done so.
 
B

Bill Cunningham

Barry said:
The current standard does not have default types for functions. The
return type for main must be specified as int.


While reaching the final } in main returns a value of 0, many consider
an explicit return statement to be the preferred style.


Make sure the program executes correctly before worrying about style.


Delete this statement since all three variables have new values
assigned in the next three statements.
OK


What should the result be when pc happens to equal low or hi. Consider
hi = 5, low = 3, and pc = 5. The range is 2 but you return -1.

You also return -1 when all three are equal even though 0 seems to be
a possible range, especially if the stock is not being traded.

Without doing any detailed checking, I think replacing the > with >=
might solve these problems.

alright. I will rewrite this. I figured I'd have some problems unforseen
by me.
 
B

Bill Cunningham

Ben said:
I don't think you need to compromise:

return fmax(hi, pc) - fmin(low, pc);

(#include <math.h> of course).

This is very good Ben. But using functions seems to be the easy way out.
I was unware of these functions but I wanted to do it manually without
depending on a function. I don't know if that's good or bad but syntax in C
is one of my major lacking points among many.

Bill
 
B

Bill Cunningham

Stephen said:
That's a decent literal translation of the requirements, except the
"else"s aren't necessary since the "then" path doesn't fall through,
and your temporary variable names aren't very informative. I also
wonder if it's possible to reach the "return -1;" path at all.

However, transforming the requirements slightly can give the correct
result with significantly less complexity and code:

double t_range(double hi, double low, double pc)
{
if (hi < pc)
hi = pc;
if (low > pc)
low = pc;
return hi - low;
}

or, for those who value brevity over clarity:

In this case that's not me. I understand the ?: is for "for" but I need
all the clarity I can get.
 
P

Paul

Bill said:
This is very good Ben. But using functions seems to be the easy way out.
I was unware of these functions but I wanted to do it manually without
depending on a function. I don't know if that's good or bad but syntax in C
is one of my major lacking points among many.

Bill

I can read Ben's code and immediately tell what it is doing.
That's important for a third party reviewing your code.

You also have the option, of taking Ben's lead, and writing
a local version of fmax() and fmin(). So you can get that
"do it yourself" feeling.

Paul
 
Ad

Advertisements

B

Bill Cunningham

Paul said:
I can read Ben's code and immediately tell what it is doing.
That's important for a third party reviewing your code.

You also have the option, of taking Ben's lead, and writing
a local version of fmax() and fmin(). So you can get that
"do it yourself" feeling.

I might try it this way. That way I know it will work anyway.

B
 
S

Stephen Sprunk

In this case that's not me. I understand the ?: is for "for" but I
need all the clarity I can get.

?: has nothing to do with "for". The below code does exactly the same
thing as the above code, just not as clearly.

S
 
B

Ben Bacarisse

Bill Cunningham said:
This is very good Ben. But using functions seems to be the easy
way out.

That's not the right perspective. Anything that aids programming just
lets you write more complex programs. Using language facilities to
their full extent (and, goodness knows, C doesn't offer you that much
help) doesn't make programming easier, it makes it easier to program
more.
I was unware of these functions but I wanted to do it manually without
depending on a function. I don't know if that's good or bad but syntax
in C is one of my major lacking points among many.

As Paul has pointed out, not knowing about these functions is not really
the issue. I think max(a, b) - min(c, b) is the right way to express
what you mean, and if that means writing max and min, so be it.

By the way, there's another clear way to express what you mean. When I
first saw your code, I was going to suggest this alternative:

double t_range(double hi, double low, double pc)
{
return fmax(fmax(hi - low, pc - low), hi - pc);
}

and I'd say it's still a good candidate.

One way or the other, this is a matter of maximums and minimums so
that's the way to write it, even if you need to write the basic
functions to do it.
 
S

Stephen Sprunk

I don't think you need to compromise:

return fmax(hi, pc) - fmin(low, pc);

(#include <math.h> of course).

*facepalm*

I'm so used to there not being standard min/max functions for integers
that I forgot they exist for floating point.

That's definitely clearer than my version above, though IMHO it's still
not quite as clear as my long version.

S
 
B

Ben Bacarisse

Stephen Sprunk said:
*facepalm*

I'm so used to there not being standard min/max functions for integers
that I forgot they exist for floating point.

That's definitely clearer than my version above, though IMHO it's still
not quite as clear as my long version.

That's interesting. You mean the one that sets hi if pc is larger, then
sets lo if pc is lower, and finally returns the difference?

I almost always find a side effect-free expression clearer and easier to
reason about than a statement sequence with side effects or an
expression with side effects. Maybe this just reflects the languages
I've ended up using over the years.

Neither is complex enough to be unclear in an any absolute sense; it's
entirely a relative judgement.
 
Ad

Advertisements

S

Stephen Sprunk

That's interesting. You mean the one that sets hi if pc is larger,
then sets lo if pc is lower, and finally returns the difference?
Yes.

I almost always find a side effect-free expression clearer and easier
to reason about than a statement sequence with side effects or an
expression with side effects. Maybe this just reflects the
languages I've ended up using over the years.

Perhaps. I've never worked in a purely functional language, which
likely lead to me having a mental model of programs as state machines
rather than as mathematical functions. I have to mentally convert the
latter to the former to understand them.
Neither is complex enough to be unclear in an any absolute sense;
it's entirely a relative judgement.

Hence "IMHO".

S
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top