passing structures among functions

Discussion in 'C Programming' started by sofeng, Mar 3, 2007.

  1. sofeng

    sofeng Guest

    sofeng, Mar 3, 2007
    #1
    1. Advertising

  2. On Fri, 2 Mar 2007, sofeng wrote:
    >
    > The following link shows a chart I created about passing structures
    > among functions. Would you review it and tell me if it requires any
    > corrections?
    >
    > http://bp2.blogger.com/_lZhqNsiakm4/Reh26hy-JHI/AAAAAAAAAAk/wvyV3Yx8gSs/s1600-h/gif_1.gif


    FYI, many readers of Usenet *will not* go to a Web page just because
    a pseudonymous poster asks them to, especially when the page has such
    an obviously machine-generated URL. (And perhaps doubly so when the
    URL is actually the URL of an HTML page, despite the ".gif" ending.
    That's just unnecessary.)

    Note to prospective Web-goers: I used 'wget' to grab the GIF, so if
    there's any malicious code on that Web page, I didn't encounter it.
    Don't think I'm endorsing the page.

    Okay, now on to your question. Mistakes ordered from most significant
    to least significant:

    * All your ".h" files are missing the include guards.
    http://en.wikipedia.org/wiki/Include_guard
    * "get_data.h" refers to DATA, so it needs to #include "defs.h".
    * "get_data.c" refers to get_data(), so it needs to #include "get_data.h".
    * "DATA", in all caps, looks like a preprocessor macro. It is better to
    use something non-macro-ish, such as "Data" or "data_t". Except...
    * The use of "typedef" is superfluous, and most comp.lang.c regulars
    will counsel you not to use it. Give the struct a tag, such as "struct
    data", and use that instead.
    * You spelled "definitions" as "definitons" in one place.
    * "float" is rarely used; "double" would be more newbie-friendly, if your
    goal is to show what real code looks like.
    * "Header files should only contain declarations." should read
    "Header files should contain only declarations." (And macro definitions,
    and comments.)
    * Burn your GIFs and use PNG instead; it's cooler. :)

    HTH,
    -Arthur
     
    Arthur J. O'Dwyer, Mar 3, 2007
    #2
    1. Advertising

  3. sofeng

    Ian Collins Guest

    Arthur J. O'Dwyer wrote:

    > Note to prospective Web-goers: I used 'wget' to grab the GIF, so if
    > there's any malicious code on that Web page, I didn't encounter it.
    > Don't think I'm endorsing the page.
    >

    Better to just avoid the windows/IE combination :)

    --
    Ian Collins.
     
    Ian Collins, Mar 3, 2007
    #3
  4. sofeng

    sofeng Guest

    On Mar 2, 5:16 pm, "Arthur J. O'Dwyer" <>
    wrote:
    > On Fri, 2 Mar 2007, sofeng wrote:
    >
    > > The following link shows a chart I created about passing structures
    > > among functions. Would you review it and tell me if it requires any
    > > corrections?

    >
    > >http://bp2.blogger.com/_lZhqNsiakm4/Reh26hy-JHI/AAAAAAAAAAk/wvyV3Yx8g...

    >
    > FYI, many readers of Usenet *will not* go to a Web page just because
    > a pseudonymous poster asks them to, especially when the page has such
    > an obviously machine-generated URL. (And perhaps doubly so when the
    > URL is actually the URL of an HTML page, despite the ".gif" ending.
    > That's just unnecessary.)
    >
    > Note to prospective Web-goers: I used 'wget' to grab the GIF, so if
    > there's any malicious code on that Web page, I didn't encounter it.
    > Don't think I'm endorsing the page.
    >
    > Okay, now on to your question. Mistakes ordered from most significant
    > to least significant:
    >
    > * All your ".h" files are missing the include guards.http://en.wikipedia.org/wiki/Include_guard
    > * "get_data.h" refers to DATA, so it needs to #include "defs.h".
    > * "get_data.c" refers to get_data(), so it needs to #include "get_data.h".
    > * "DATA", in all caps, looks like a preprocessor macro. It is better to
    > use something non-macro-ish, such as "Data" or "data_t". Except...
    > * The use of "typedef" is superfluous, and most comp.lang.c regulars
    > will counsel you not to use it. Give the struct a tag, such as "struct
    > data", and use that instead.
    > * You spelled "definitions" as "definitons" in one place.
    > * "float" is rarely used; "double" would be more newbie-friendly, if your
    > goal is to show what real code looks like.
    > * "Header files should only contain declarations." should read
    > "Header files should contain only declarations." (And macro definitions,
    > and comments.)
    > * Burn your GIFs and use PNG instead; it's cooler. :)
    >
    > HTH,
    > -Arthur


    Thank you much for the feedback. I have made the changes suggested.
    How does it look now? Here is the link for the png file:
    http://bp1.blogger.com/_lZhqNsiakm4/RezJubUauVI/AAAAAAAAAA4/nYIxnx9E1Z8/s1600-h/png_1.png
    The text contained in the diagram is below:

    main.c:
    - main.c contains the main function which calls the other functions
    that operate on the data structure.
    - It also defines (allocates memory) for the data structure at the
    file level.
    - The data structure is declared using the static keyword so that it
    will have static duration (i.e. it will exist from program start to
    finish), but it will have internal linkage (i.e. it won't be global).
    - The address operator, &, is used to create a pointer to the data
    structure which is passed to the other functions.

    #include "defs.h"
    #include "get_data.h"
    #include "modify_data.h"

    static struct data the_data;

    int main()
    {
    get_data(&the_data);
    modify_data(&the_data);

    return 0;
    }

    get_data.c:
    - get_data() fills the data structure
    - it is passed a pointer to the data structure defined in main.c

    #include "defs.h"
    #include "get_data.h"

    int get_data(struct data *data_ptr)
    {
    data_ptr->item1 = 1;
    data_ptr->item2 = 2;
    data_ptr->item3 = 3.0;
    data_ptr->item4 = 4.0;

    return 0;
    }

    modify_data.c:
    - modify_data() modifies the data structure
    - It is similar to get_data()

    #include "defs.h"
    #include "modify_data.h"

    int modify_data(struct data *data_ptr)
    {
    data_ptr->item2 += 1;
    data_ptr->item4 += 1.0;

    return 0;
    }

    defs.h:
    - contains the structure data type declaration

    #ifndef DEFS_H_
    #define DEFS_H_

    struct data
    {
    int item1;
    int item2;
    double item3;
    double item4;
    };

    #endif /*DEFS_H_*/

    get_data.h:
    - contains the get_data() function declaration

    #ifndef GET_DATA_H_
    #define GET_DATA_H_

    #include "defs.h"

    int get_data(struct data *data_ptr);

    #endif /*GET_DATA_H_*/

    modify_data.h:
    - contains the modify_data() function declaration

    #ifndef MODIFY_DATA_H_
    #define MODIFY_DATA_H_

    #include "defs.h"

    int modify_data(struct data *data_ptr);

    #endif /*MODIFY_DATA_H_*/

    NOTE: Header files should contain only declarations, macro
    definitions, and comments. They should not contain variable
    definitions because if the header file is included in multiple
    locations, there would be multiple definitions of the same variable.
     
    sofeng, Mar 6, 2007
    #4
  5. sofeng

    Flash Gordon Guest

    sofeng wrote, On 06/03/07 02:07:

    <snip>

    > Thank you much for the feedback. I have made the changes suggested.
    > How does it look now? Here is the link for the png file:
    > http://bp1.blogger.com/_lZhqNsiakm4/RezJubUauVI/AAAAAAAAAA4/nYIxnx9E1Z8/s1600-h/png_1.png
    > The text contained in the diagram is below:


    I'm still not following the link (too lazy) but since you've included
    the code it does not matter :)

    > main.c:
    > - main.c contains the main function which calls the other functions
    > that operate on the data structure.
    > - It also defines (allocates memory) for the data structure at the
    > file level.
    > - The data structure is declared using the static keyword so that it
    > will have static duration (i.e. it will exist from program start to
    > finish), but it will have internal linkage (i.e. it won't be global).
    > - The address operator, &, is used to create a pointer to the data
    > structure which is passed to the other functions.
    >
    > #include "defs.h"
    > #include "get_data.h"
    > #include "modify_data.h"
    >
    > static struct data the_data;


    There is no need for this to be a file scope variable. Better would be
    to declare it inside main and not static. This will also "hide" it from
    any other functions you (or someone else) later write in the source file
    containing it.

    > int main()


    As you are not using command line parameters better to be explicit
    int main(void)

    > {

    struct data the_data;
    Or, if you want it to be initialised
    struct data the_data = {0};

    > get_data(&the_data);
    > modify_data(&the_data);
    >
    > return 0;
    > }
    >
    > get_data.c:
    > - get_data() fills the data structure
    > - it is passed a pointer to the data structure defined in main.c
    >
    > #include "defs.h"
    > #include "get_data.h"
    >
    > int get_data(struct data *data_ptr)


    Since this only ever returns the one value why have it return anything
    at all? Better would be
    void get_data(struct data *data_ptr)
    Then loose the return at the bottom.

    > {
    > data_ptr->item1 = 1;
    > data_ptr->item2 = 2;
    > data_ptr->item3 = 3.0;
    > data_ptr->item4 = 4.0;
    >
    > return 0;
    > }
    >
    > modify_data.c:
    > - modify_data() modifies the data structure
    > - It is similar to get_data()
    >
    > #include "defs.h"
    > #include "modify_data.h"
    >
    > int modify_data(struct data *data_ptr)


    Same comment as last one. Better
    void modify_data(struct data *data_ptr)

    <snip stuff that looked OK at a glance>

    > NOTE: Header files should contain only declarations, macro
    > definitions, and comments. They should not contain variable
    > definitions because if the header file is included in multiple
    > locations, there would be multiple definitions of the same variable.


    typedefs are also OK in headers when it makes sense to use them, which
    it does sometimes.
    --
    Flash Gordon
     
    Flash Gordon, Mar 6, 2007
    #5
    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. ILLOGIC
    Replies:
    1
    Views:
    382
    Rob Williscroft
    Jun 1, 2004
  2. Xiangliang Meng
    Replies:
    1
    Views:
    1,662
    Victor Bazarov
    Jun 21, 2004
  3. Steven T. Hatton

    Passing member functions to C functions?

    Steven T. Hatton, Oct 4, 2004, in forum: C++
    Replies:
    7
    Views:
    1,342
    David Hilsee
    Oct 7, 2004
  4. tweak
    Replies:
    14
    Views:
    2,818
    Eric Sosman
    Jun 11, 2004
  5. Alfonso Morra
    Replies:
    11
    Views:
    753
    Emmanuel Delahaye
    Sep 24, 2005
Loading...

Share This Page