Thread: Organizing my code in C

  1. #1
    Registered User
    Join Date
    Apr 2008
    Posts
    87

    Organizing my code in C

    Can some one please provide suggestions as to how I can improve the
    organization of main file ?

    In my ray tracing project, I have to carry out following task (in
    sequence)

    1. Read the mesh from an ascii file and store it in the mesh data
    structure.
    2. Create the kd tree for fast mesh traversal.
    4. Create rays, trace them on the fly and calculate the scattered and incident
    electric fields.

    currently it looks like this:

    Code:
    #include "initialize.h"
    #include "mesh.h"
    #include "vector.h"
    #include "kdtree.h"
    #include "common.h"
    #include "ray.h"
    #include <stdio.h>
    #include <stdlib.h>
     
    
    /* read_mesh, init_plwave and create_kdtree are functions defined in initialize.c */
    /* which basically open mesh file, allocate memory, scan some data from user etc */
    /* and finally call the program which does the actual processing on the data structure */
    
    int initialize(mesh **m, char *zfname, kdtree **tree)
    {
      
      if(read_mesh(zfname, m) || init_plwave() || create_kdtree(tree, *m))
      {
        killall(); /* destroy all data structures */
        return (1);
      }
      return (0);
    }
    
    int main(void)
    {
      mesh *m = NULL; /* pointer to mesh */
      kdtree *tree = NULL; /* pointer to kd tree */
      char *ptr = NULL; 
      char filename[50];
    
      printf("Enter the file name\n");
      if(fgets(filename, sizeof(filename), stdin) == NULL)
      {
        error_report("Bad file name");
        exit(1);
      }
    
      if((ptr = strchr(filename, '\n')) != NULL)
        *ptr = '\0';
       
      if(initialize(&m, filename, &tree))
        exit(1);
    
     if(calc(m, tree))  /* do calculations */
     {
       killall();
       return (1);
     }
    
      print_result(); /* display result */  
    
      killall(); /* destroy all data structures */
    
      exit(0);
    }

  2. #2
    Registered User
    Join Date
    Apr 2008
    Posts
    87
    Btw I'm planning to declare m, tree, filename as extern variables in common.h and define them in main.c because they are to persist in memory as long as the calculations are being done. Also, once they are initialized, they won't be changed during calculations. Is it really bad to use extern even if you know what you want to do.
    Last edited by broli86; 06-27-2008 at 04:03 PM.

  3. #3
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,613
    >> I'm planning to declare m, tree as extern variables because they are to persist in
    >> memory as long as the calculations are being done

    That would be the wrong reason since extern has no bearing on the scope of a variable. extern tells the linker to look elsewhere for a variable because it is actually declared in another module.

    If the source does what it's meant to there's not much to do here.

    Why is exit() so appropriate for main() rather than a simple return statement?

    Are you comfortable with how initialize_mesh() is written or could it be refactored? I'm not sure what those functions do but I would rather break it into several statements rather than chain them together with or:

    Code:
    int rv_read = read_mesh(zfname, m);
    int rv_init = init_plwave();
    int rv_tree = create_kdtree(tree, *m)
    if(rv_read || rv_init || rv_tree )
      {
        killall(); /* destroy all data structures */
        return (1);
      }
    It's simply cleaner because the functions would always be called top down and their return values could still be used to clean up. But whatever makes sense.

  4. #4
    Registered User
    Join Date
    Apr 2008
    Posts
    87
    Quote Originally Posted by citizen View Post
    >> I'm planning to declare m, tree as extern variables because they are to persist in
    >> memory as long as the calculations are being done

    That would be the wrong reason since extern has no bearing on the scope of a variable. extern tells the linker to look elsewhere for a variable because it is actually declared in another module.

    If the source does what it's meant to there's not much to do here.

    Why is exit() so appropriate for main() rather than a simple return statement?

    Are you comfortable with how initialize_mesh() is written or could it be refactored? I'm not sure what those functions do but I would rather break it into several statements rather than chain them together with or:

    Code:
    int rv_read = read_mesh(zfname, m);
    int rv_init = init_plwave();
    int rv_tree = create_kdtree(tree, *m)
    if(rv_read || rv_init || rv_tree )
      {
        killall(); /* destroy all data structures */
        return (1);
      }
    It's simply cleaner because the functions would always be called top down and their return values could still be used to clean up. But whatever makes sense.
    what if its done *properly* ?
    Well these data structures are frequently needed in many functions, and continously passing them everywhere is extermely painful even when they are hardly used in a function. It has made my code extermely obfuscated, difficult to read and probably even slower in cases where recursive functions are used. Let's say mesh *m is passed to a recursive function. Then in that case, for all the calls a seperate copy of m will be maintained even if m was utilized in probably 1 statement. Apart from this during the erro handling, it is quite easy to call the killall function from any where in the program where the error occured and free the data structures allocated upto that point. No need to pass any parameters.

    eg :

    Code:
    void killall()
    {
      if(m != NULL)
        killlmesh();
      if(tree != NULL)
        killtree();
    }
    Last edited by broli86; 06-27-2008 at 04:45 PM.

  5. #5
    Registered User
    Join Date
    Sep 2006
    Posts
    8,868
    The purists may moan, but if a data structure is used in almost every function, it seems natural to make it global, in the program. That's the purpose of global scope, imo.

    Recursive calls with a non-global array. would only be passing a pointer, no matter how big the array was.

    Nevertheless, an array that is used by nearly every function, should be carefully considered as a global array, at least.

  6. #6
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    I personally dislike returning true to indicate failure as it makes the progam logic seem wrong. I also second the point about simply using return statements from main. However it seems otherwise okay.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  7. #7
    Registered User
    Join Date
    Apr 2008
    Posts
    87
    What about the error flag ? How about making it extern as well instead of propogating it every where and then back to main ?

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Enforcing Machine Code Restrictions?
    By SMurf in forum Tech Board
    Replies: 21
    Last Post: 03-30-2009, 07:34 AM
  2. Obfuscated Code Contest: The Results
    By Stack Overflow in forum Contests Board
    Replies: 29
    Last Post: 02-18-2005, 05:39 PM
  3. Obfuscated Code Contest
    By Stack Overflow in forum Contests Board
    Replies: 51
    Last Post: 01-21-2005, 04:17 PM
  4. Interface Question
    By smog890 in forum C Programming
    Replies: 11
    Last Post: 06-03-2002, 05:06 PM
  5. Replies: 0
    Last Post: 02-21-2002, 06:05 PM