stylistic question

[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

[Stephen Sprunk]

On 10-Nov-12 15:24, Bill Cunningham wrote:
> 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

--
Stephen Sprunk "God does not play dice." --Albert Einstein
CCIE #3723 "God is an inveterate gambler, and He throws the
K5SSS dice at every possible opportunity." --Stephen Hawking

[Ike Naar]

On 2012-11-10, Stephen Sprunk <ste...@sprunk.org> wrote:
> On 10-Nov-12 15:24, Bill Cunningham wrote:
>> #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;
>> }
>> }

> 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.

[Barry Schwarz]

On Sat, 10 Nov 2012 16:24:58 -0500, "Bill Cunningham"
<nos...@nspam.invalid> wrote:

> 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.

> printf("%.2f\n", r);
> return 0;
>}

--
Remove del for email

[Stephen Sprunk]

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

[Ben Bacarisse]

Stephen Sprunk <ste...@sprunk.org> writes:
<snip>

> 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).

--
Ben.

[Keith Thompson]

"Bill Cunningham" <nos...@nspam.invalid> writes:
> 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.

--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
Will write code for food.
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

[Bill Cunningham]

Barry Schwarz wrote:
> On Sat, 10 Nov 2012 16:24:58 -0500, "Bill Cunningham"
> <nos...@nspam.invalid> wrote:
>
>> 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.

OK

>> 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.

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

[Bill Cunningham]

Ben Bacarisse wrote:

> 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

[Bill Cunningham]

Stephen Sprunk wrote:

> 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.

[Paul]

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

[Bill Cunningham]

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

B

[Stephen Sprunk]

On 11-Nov-12 13:10, Bill Cunningham wrote:
> Stephen Sprunk wrote:
>> 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.

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

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

S

[Ben Bacarisse]

"Bill Cunningham" <nos...@nspam.invalid> writes:

> Ben Bacarisse wrote:
>
>> 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.

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.

--
Ben.

[Stephen Sprunk]

*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.

[Ben Bacarisse]

Stephen Sprunk <ste...@sprunk.org> writes:

> On 10-Nov-12 21:17, Ben Bacarisse wrote:
>> Stephen Sprunk <ste...@sprunk.org> writes:
>> <snip>
>>> 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).
>
> *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.

--
Ben.

[Stephen Sprunk]

On 11-Nov-12 16:13, Ben Bacarisse wrote:
> Stephen Sprunk <ste...@sprunk.org> writes:
>> On 10-Nov-12 21:17, Ben Bacarisse wrote:
>>> I don't think you need to compromise:
>>>
>>> return fmax(hi, pc) - fmin(low, pc);
>>

>> ...

>> 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?

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".