stylistic question

Discussion in 'C Programming' started by Bill Cunningham, Nov 10, 2012.

  1. 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
    Bill Cunningham, Nov 10, 2012
    #1
    1. Advertising

  2. 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
    Stephen Sprunk, Nov 10, 2012
    #2
    1. Advertising

  3. Bill Cunningham

    Ike Naar Guest

    On 2012-11-10, Stephen Sprunk <> 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.
    Ike Naar, Nov 10, 2012
    #3
  4. On Sat, 10 Nov 2012 16:24:58 -0500, "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


    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
    Barry Schwarz, Nov 10, 2012
    #4
  5. On 10-Nov-12 17:22, Ike Naar wrote:
    > On 2012-11-10, Stephen Sprunk <> 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.


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

    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
    Stephen Sprunk, Nov 11, 2012
    #5
  6. Stephen Sprunk <> 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.
    Ben Bacarisse, Nov 11, 2012
    #6
  7. "Bill Cunningham" <> 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) <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"
    Keith Thompson, Nov 11, 2012
    #7
  8. Barry Schwarz wrote:
    > On Sat, 10 Nov 2012 16:24:58 -0500, "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

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

    >> }
    >> }
    >>
    >> 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;
    >> }
    Bill Cunningham, Nov 11, 2012
    #8
  9. 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, Nov 11, 2012
    #9
  10. 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.

    > double t_range(double hi, double low, double pc)
    > {
    > return (pc > hi ? pc : hi) - (pc < low ? pc : low);
    > }
    >
    >
    > S
    Bill Cunningham, Nov 11, 2012
    #10
  11. Bill Cunningham

    Paul Guest

    Bill Cunningham wrote:
    > 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


    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
    Paul, Nov 11, 2012
    #11
  12. Paul wrote:
    > Bill Cunningham wrote:
    >> 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

    >
    > 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
    Bill Cunningham, Nov 11, 2012
    #12
  13. 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

    --
    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
    Stephen Sprunk, Nov 11, 2012
    #13
  14. "Bill Cunningham" <> 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.
    Ben Bacarisse, Nov 11, 2012
    #14
  15. On 10-Nov-12 21:17, Ben Bacarisse wrote:
    > Stephen Sprunk <> 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.

    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
    Stephen Sprunk, Nov 11, 2012
    #15
  16. Stephen Sprunk <> writes:

    > On 10-Nov-12 21:17, Ben Bacarisse wrote:
    >> Stephen Sprunk <> 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.
    Ben Bacarisse, Nov 11, 2012
    #16
  17. On 11-Nov-12 16:13, Ben Bacarisse wrote:
    > Stephen Sprunk <> 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".

    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
    Stephen Sprunk, Nov 12, 2012
    #17
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. P.Hill
    Replies:
    13
    Views:
    541
    Adam Maass
    Apr 23, 2004
  2. Denis Remezov
    Replies:
    4
    Views:
    462
    Siemel Naran
    Apr 30, 2004
  3. Mantorok Redgormor

    stylistic prototype question

    Mantorok Redgormor, Oct 29, 2003, in forum: C Programming
    Replies:
    5
    Views:
    345
    Mark A. Odell
    Oct 30, 2003
  4. Andrew Koenig

    Stylistic question about inheritance

    Andrew Koenig, Mar 31, 2005, in forum: Python
    Replies:
    17
    Views:
    470
    Guy Bolton King
    Apr 1, 2005
  5. Mark Healey

    A stylistic question.

    Mark Healey, May 14, 2006, in forum: C Programming
    Replies:
    60
    Views:
    1,192
    Jordan Abel
    May 28, 2006
Loading...

Share This Page