Style question

Discussion in 'C Programming' started by Nick, Oct 19, 2010.

  1. Nick

    Nick Guest

    I'm not after a definitive answer here - I know there aren't any. But
    I'm curious to know how people feel about something.

    A few days ago I found myself writing something like this:

    find_item(name)->identifier

    Where find_item looks up a name in a data structure and returns a
    pointer to the node (a struct). The "identifier" is an integer.

    I was, in fact, using this as a paramater to another function, that took
    an integer identifier and did some more stuff with it.

    There's a bit of me that thinks this is really neat, and another bit of
    me that found the resulting three-line statement horrible.

    In the end I realised I needed different items at different times, so
    introduced a temporary variable. But how do others feel about this
    idea of getting a struct pointer back from a function and extracting a
    member directly from it?
    --
    Online waterways route planner | http://canalplan.eu
    Plan trips, see photos, check facilities | http://canalplan.org.uk
     
    Nick, Oct 19, 2010
    #1
    1. Advertising

  2. Nick

    Felix Palmen Guest

    * Nick <>:
    > I'm not after a definitive answer here - I know there aren't any. But
    > I'm curious to know how people feel about something.
    >
    > A few days ago I found myself writing something like this:
    >
    > find_item(name)->identifier
    >
    > Where find_item looks up a name in a data structure and returns a
    > pointer to the node (a struct). The "identifier" is an integer.
    >
    > I was, in fact, using this as a paramater to another function, that took
    > an integer identifier and did some more stuff with it.
    >
    > There's a bit of me that thinks this is really neat, and another bit of
    > me that found the resulting three-line statement horrible.
    >
    > In the end I realised I needed different items at different times, so
    > introduced a temporary variable. But how do others feel about this
    > idea of getting a struct pointer back from a function and extracting a
    > member directly from it?


    I'd put it a little more generally and discuss chained and nested
    function calls. And I know this tradeoff from my own experience:

    Given proper names and logical nesting/chaining that really describes
    the intent of the statement, it just looks quite elegant.

    The downside is IMO twofold:

    - As your program evolves, you may need to add some functionality in
    your already complex statement, missing the point where it turns from
    being just obvious, nice and clean into some incomprehensible monster.
    - When debugging, you're missing the opportunity to easily single-step
    over each involved function, checking the intermediate results.

    So, most of the time I'd vote against to much chaining/nesting of
    function calls.

    Regards,
    Felix

    --
    Felix Palmen (Zirias) + [PGP] Felix Palmen <>
    web: http://palmen-it.de/ | http://palmen-it.de/pub.txt
    my open source projects: | Fingerprint: ED9B 62D0 BE39 32F9 2488
    http://palmen-it.de/?pg=pro + 5D0C 8177 9D80 5ECF F683
     
    Felix Palmen, Oct 19, 2010
    #2
    1. Advertising

  3. Nick

    Seebs Guest

    On 2010-10-19, Nick <> wrote:
    > In the end I realised I needed different items at different times, so
    > introduced a temporary variable. But how do others feel about this
    > idea of getting a struct pointer back from a function and extracting a
    > member directly from it?


    Is it remotely, ever, conceivable that the function will return NULL?

    -s
    --
    Copyright 2010, all wrongs reversed. Peter Seebach /
    http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
    http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!
    I am not speaking for my employer, although they do rent some of my opinions.
     
    Seebs, Oct 19, 2010
    #3
  4. Nick <> might have writ, in
    news::

    > find_item(name)->identifier


    Perhaps even more problematic is
    copy_item(name).identifier

    There are some restrictions on such constructions;
    perhaps someone can post an "executive summary."

    James
     
    James Dow Allen, Oct 19, 2010
    #4
  5. Nick

    Ian Collins Guest

    On 10/19/10 09:30 PM, James Dow Allen wrote:
    > Nick<> might have writ, in
    > news::
    >
    >> find_item(name)->identifier

    >
    > Perhaps even more problematic is
    > copy_item(name).identifier


    Probably less problematic, copy_item can't return null.

    --
    Ian Collins
     
    Ian Collins, Oct 19, 2010
    #5
  6. Nick

    Tom St Denis Guest

    On Oct 19, 2:28 am, Nick <> wrote:
    > In the end I realised I needed different items at different times, so
    > introduced a temporary variable.  But how do others feel about this
    > idea of getting a struct pointer back from a function and extracting a
    > member directly from it?


    I'd avoid that solely on the premise that function could potentially
    fail [return NULL] and thus would segfault your application.

    The typical way I would do that is

    item = find_item(name);
    if (item == NULL) { return error_code_of_some_sort; }
    // ... use item

    If it's a short usage you might get away with

    if ((item = find_item(name)) != NULL) {
    // use item
    } else {
    // return error
    }

    but if you subsequently "find" another item the code can get nested
    crazy deep and weird.

    Tom
     
    Tom St Denis, Oct 19, 2010
    #6
  7. Nick

    ImpalerCore Guest

    On Oct 19, 2:28 am, Nick <> wrote:
    > I'm not after a definitive answer here - I know there aren't any.  But
    > I'm curious to know how people feel about something.
    >
    > A few days ago I found myself writing something like this:
    >
    > find_item(name)->identifier
    >
    > Where find_item looks up a name in a data structure and returns a
    > pointer to the node (a struct).  The "identifier" is an integer.
    >
    > I was, in fact, using this as a paramater to another function, that took
    > an integer identifier and did some more stuff with it.
    >
    > There's a bit of me that thinks this is really neat, and another bit of
    > me that found the resulting three-line statement horrible.
    >
    > In the end I realised I needed different items at different times, so
    > introduced a temporary variable.  But how do others feel about this
    > idea of getting a struct pointer back from a function and extracting a
    > member directly from it?


    In general, any function that returns a struct pointer with a 'find'
    or 'search' in the function name is likely to return NULL eventually,
    as NULL typically represents the 'not found' return value semantic of
    the lookup. To me, this is a bomb with a relatively short fuse. If
    the maintenance programmer that follows you finds a bug as a result of
    'find_item(name)->identifier', I doubt your cleverness will be
    appreciated.

    Best regards,
    John D.
     
    ImpalerCore, Oct 19, 2010
    #7
  8. Nick

    Tom St Denis Guest

    On Oct 19, 4:46 am, Ian Collins <> wrote:
    > On 10/19/10 09:30 PM, James Dow Allen wrote:
    >
    > > Nick<>  might have writ, in
    > >news::

    >
    > >> find_item(name)->identifier

    >
    > > Perhaps even more problematic is
    > >      copy_item(name).identifier

    >
    > Probably less problematic, copy_item can't return null.


    Nah, it's just a performance hazard and what you're referencing could
    be NULL e.g.

    printf("name is %s\n", find_item(ID).name);

    Brilliant! Combining two problems into one!
     
    Tom St Denis, Oct 19, 2010
    #8
  9. Nick

    Willem Guest

    Kenneth Brody wrote:
    ) On the other hand, sometimes the resulting code of directly using the
    ) returned pointer just looks awkward.
    )
    ) foo(bar(plugh)->item[n].addr)->name = "foo";

    This is only awkward if you're not used to functional programming.


    SaSW, Willem
    --
    Disclaimer: I am in no way responsible for any of the statements
    made in the above text. For all I know I might be
    drugged or something..
    No I'm not paranoid. You all think I'm paranoid, don't you !
    #EOT
     
    Willem, Oct 19, 2010
    #9
  10. Nick

    Nick Guest

    Seebs <> writes:

    > On 2010-10-19, Nick <> wrote:
    >> In the end I realised I needed different items at different times, so
    >> introduced a temporary variable. But how do others feel about this
    >> idea of getting a struct pointer back from a function and extracting a
    >> member directly from it?

    >
    > Is it remotely, ever, conceivable that the function will return NULL?


    Not in this case. But I agree that that is a perfectly natural thing
    for a search function to do, and completely breaks this approach.
    --
    Online waterways route planner | http://canalplan.eu
    Plan trips, see photos, check facilities | http://canalplan.org.uk
     
    Nick, Oct 19, 2010
    #10
  11. Nick

    Nick Guest

    Kenneth Brody <> writes:

    > On the other hand, sometimes the resulting code of directly using the
    > returned pointer just looks awkward.
    >
    > foo(bar(plugh)->item[n].addr)->name = "foo";


    And, indeed, that was just the sort of mess I ended up with.
    --
    Online waterways route planner | http://canalplan.eu
    Plan trips, see photos, check facilities | http://canalplan.org.uk
     
    Nick, Oct 19, 2010
    #11
  12. Nick

    Jorgen Grahn Guest

    On Tue, 2010-10-19, Nick wrote:
    > Seebs <> writes:
    >
    >> On 2010-10-19, Nick <> wrote:
    >>> In the end I realised I needed different items at different times, so
    >>> introduced a temporary variable. But how do others feel about this
    >>> idea of getting a struct pointer back from a function and extracting a
    >>> member directly from it?

    >>
    >> Is it remotely, ever, conceivable that the function will return NULL?

    >
    > Not in this case. But I agree that that is a perfectly natural thing
    > for a search function to do, and completely breaks this approach.


    find_item(name)->identifier

    That word "find" is actually my only objection. It hints that you're
    not quite sure you'll find anything (or it would have been called
    get_item() or simply item()).

    /Jorgen

    --
    // Jorgen Grahn <grahn@ Oo o. . .
    \X/ snipabacken.se> O o .
     
    Jorgen Grahn, Oct 19, 2010
    #12
  13. Nick

    Jorgen Grahn Guest

    On Tue, 2010-10-19, Felix Palmen wrote:
    ....
    > I'd put it a little more generally and discuss chained and nested
    > function calls. And I know this tradeoff from my own experience:
    >
    > Given proper names and logical nesting/chaining that really describes
    > the intent of the statement, it just looks quite elegant.
    >
    > The downside is IMO twofold:
    >
    > - As your program evolves, you may need to add some functionality in
    > your already complex statement, missing the point where it turns from
    > being just obvious, nice and clean into some incomprehensible monster.


    I object to that. You need to learn to detect that point and to act on
    it when you detect it /anyway/.

    Code should IMHO not be structured to be prepared for some
    hypothetical future extension. It's better to do that work
    ("refactoring") when there's an immediate need for it.

    > - When debugging, you're missing the opportunity to easily single-step
    > over each involved function, checking the intermediate results.


    Can't modern debuggers handle that? I must admit that I never use
    stepping symbolic deuggers.

    /Jorgen

    --
    // Jorgen Grahn <grahn@ Oo o. . .
    \X/ snipabacken.se> O o .
     
    Jorgen Grahn, Oct 19, 2010
    #13
  14. Nick

    Felix Palmen Guest

    * Jorgen Grahn <>:
    >> - As your program evolves, you may need to add some functionality in
    >> your already complex statement, missing the point where it turns from
    >> being just obvious, nice and clean into some incomprehensible monster.

    >
    > I object to that. You need to learn to detect that point and to act on
    > it when you detect it /anyway/.


    Sure you /should/, but I guess you know it happens.

    > Code should IMHO not be structured to be prepared for some
    > hypothetical future extension. It's better to do that work
    > ("refactoring") when there's an immediate need for it.


    And it's best to avoid constructs that make refactoring unnecessarily
    hard. At least as long as they don't provide some other benefit.

    But I didn't say I dislike chaining and nesting in any case. Those were
    just some issues to think about.

    >> - When debugging, you're missing the opportunity to easily single-step
    >> over each involved function, checking the intermediate results.

    >
    > Can't modern debuggers handle that? I must admit that I never use
    > stepping symbolic deuggers.


    Maybe I'm missing some feature, but I didn't succeed to display an
    intermediate result that is not assigned to a variable in gdb as well as
    the microsoft debugger. For the single stepping -- at least using
    microsoft's debugger in visual studio, this works, but it's pretty
    pointless as long as you don't see a function's return value...

    Regards,
    Felix

    --
    Felix Palmen (Zirias) + [PGP] Felix Palmen <>
    web: http://palmen-it.de/ | http://palmen-it.de/pub.txt
    my open source projects: | Fingerprint: ED9B 62D0 BE39 32F9 2488
    http://palmen-it.de/?pg=pro + 5D0C 8177 9D80 5ECF F683
     
    Felix Palmen, Oct 19, 2010
    #14
  15. Nick

    ImpalerCore Guest

    On Oct 19, 2:15 pm, Nick <> wrote:
    > Seebs <> writes:
    > > On 2010-10-19, Nick <> wrote:
    > >> In the end I realised I needed different items at different times, so
    > >> introduced a temporary variable.  But how do others feel about this
    > >> idea of getting a struct pointer back from a function and extracting a
    > >> member directly from it?

    >
    > > Is it remotely, ever, conceivable that the function will return NULL?

    >
    > Not in this case.  But I agree that that is a perfectly natural thing
    > for a search function to do, and completely breaks this approach.


    Someone else mentioned it as well, but I would avoid using 'find' in
    that function name since most of my experience with functions
    containing 'find' or 'search' imply the possibility of returning
    NULL. If I wasn't familiar with the function, and didn't read the
    documentation, I would likely assume that NULL is a possibility and
    automatically add code to guard against NULL dereferencing.

    Using 'get' or 'lookup' feels better to me, but these are just my
    opinions.

    Best regards,
    John D.
     
    ImpalerCore, Oct 19, 2010
    #15
  16. On 2010-10-19, Felix Palmen <> wrote:
    > * Jorgen Grahn <>:
    >>> - When debugging, you're missing the opportunity to easily single-step
    >>> over each involved function, checking the intermediate results.

    >>
    >> Can't modern debuggers handle that? I must admit that I never use
    >> stepping symbolic deuggers.

    >
    > Maybe I'm missing some feature, but I didn't succeed to display an
    > intermediate result that is not assigned to a variable in gdb as well as
    > the microsoft debugger. For the single stepping -- at least using
    > microsoft's debugger in visual studio, this works, but it's pretty
    > pointless as long as you don't see a function's return value...


    FWIW, gdb's "finish" command displays the return value, but it is a bit
    cumbersome: you need to single-step in, and then "finish" out, like this:

    $ cat > test.c
    int func(x) { return x + 42; }
    int main(void) { return func(42); }

    Breakpoint 1, main () at test.c:2
    2 int main(void) { return func(42); }
    (gdb) step
    func (x=42) at test.c:1
    1 int func(x) { return x + 42; }
    (gdb) fini
    Run till exit from #0 func (x=42) at test.c:1
    0x00000000004004d6 in main () at test.c:2
    2 int main(void) { return func(42); }
    Value returned is $1 = 84
    (gdb) print $1
    $2 = 84

    On the positive side, you do get the return value and it is stored into
    a gdb variable for further manipulations.

    --
    Heikki Kallasjoki
    email: echo 'zfs+es_t_i@n_u.zf' | tr zen_muftis fuze_mints
     
    Heikki Kallasjoki, Oct 19, 2010
    #16
  17. Nick

    Felix Palmen Guest

    * Heikki Kallasjoki <>:
    > FWIW, gdb's "finish" command displays the return value, but it is a bit
    > cumbersome: you need to single-step in, and then "finish" out, like this:


    Nice, thank you; I'll safe that for later reference.

    Regards,
    Felix

    --
    Felix Palmen (Zirias) + [PGP] Felix Palmen <>
    web: http://palmen-it.de/ | http://palmen-it.de/pub.txt
    my open source projects: | Fingerprint: ED9B 62D0 BE39 32F9 2488
    http://palmen-it.de/?pg=pro + 5D0C 8177 9D80 5ECF F683
     
    Felix Palmen, Oct 19, 2010
    #17
  18. On Oct 19, 11:04 am, Kenneth Brody <> wrote:

    > On the other hand, sometimes the resulting code of directly using the
    > returned pointer just looks awkward.
    >
    >      foo(bar(plugh)->item[n].addr)->name = "foo";



    I little whitespace helps a little:

    foo( bar(plugh)->item[n].addr )->name = "foo";

    or even:

    foo(
    bar(plugh)->item[n].addr
    )->name = "foo";

    --
    all lines should be blurry
     
    luser- -droog, Oct 20, 2010
    #18
  19. Nick

    Nick Guest

    ImpalerCore <> writes:

    > On Oct 19, 2:15 pm, Nick <> wrote:
    >> Seebs <> writes:
    >> > On 2010-10-19, Nick <> wrote:
    >> >> In the end I realised I needed different items at different times, so
    >> >> introduced a temporary variable.  But how do others feel about this
    >> >> idea of getting a struct pointer back from a function and extracting a
    >> >> member directly from it?

    >>
    >> > Is it remotely, ever, conceivable that the function will return NULL?

    >>
    >> Not in this case.  But I agree that that is a perfectly natural thing
    >> for a search function to do, and completely breaks this approach.

    >
    > Someone else mentioned it as well, but I would avoid using 'find' in
    > that function name since most of my experience with functions
    > containing 'find' or 'search' imply the possibility of returning
    > NULL. If I wasn't familiar with the function, and didn't read the
    > documentation, I would likely assume that NULL is a possibility and
    > automatically add code to guard against NULL dereferencing.
    >
    > Using 'get' or 'lookup' feels better to me, but these are just my
    > opinions.


    To be honest, find was a poor example, for just that reason, when I
    invented my example.

    Here's the actual code I'd first written:

    Set_Lookup_String(state->parent->index->holds,V_LINK_NOSKIP,
    Radix_Key_To_Id(First_Non_Skippable(state->link->place[1],
    state->link, state->skip_things)->id));

    Glossary available on request
    --
    Online waterways route planner | http://canalplan.eu
    Plan trips, see photos, check facilities | http://canalplan.org.uk
     
    Nick, Oct 20, 2010
    #19
  20. Nick

    Paul N Guest

    On 20 Oct, 02:13, luser- -droog <> wrote:
    > On Oct 19, 11:04 am, Kenneth Brody <> wrote:
    >
    > > On the other hand, sometimes the resulting code of directly using the
    > > returned pointer just looks awkward.

    >
    > >      foo(bar(plugh)->item[n].addr)->name = "foo";

    >
    > I little whitespace helps a little:
    >
    >      foo( bar(plugh)->item[n].addr )->name = "foo";
    >
    > or even:
    >
    >      foo(
    >          bar(plugh)->item[n].addr
    >          )->name = "foo";
    >


    I think I'd go instead for having a load of intermediate values, with
    descriptive names. Perhaps I'm just a wimp.
     
    Paul N, Oct 20, 2010
    #20
    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. Rob Nicholson
    Replies:
    3
    Views:
    755
    Rob Nicholson
    May 28, 2005
  2. Replies:
    0
    Views:
    2,467
  3. Replies:
    1
    Views:
    794
    Bertilo Wennergren
    Nov 24, 2003
  4. Hardeep Rakhra
    Replies:
    8
    Views:
    644
    Hardeep Rakhra
    Jan 15, 2004
  5. Ken Varn
    Replies:
    0
    Views:
    473
    Ken Varn
    Apr 26, 2004
Loading...

Share This Page