Which programming method is better?

Discussion in 'C Programming' started by Amandil, Feb 28, 2008.

  1. Amandil

    Amandil Guest

    Hi, I was wondering if someone could give me some advice. This
    question is not necessarily C-specific, but it is the language I am
    using, and I am unsure about one point in C.

    In my program, I want to process either the stdin or the list of files
    on the command line. I have two ways of doing this, and I'm asking if
    there is any advantage to doing it one way or the other.

    Method 1: (using C pseudo-code)
    FILE *fp = stdin;
    char *filename = "standard input";
    do {
    if (argv[0] != NULL) {
    filename = *argv++;
    fp = fopen(filename, "r");
    if (!fp) {
    perror(filename);
    continue; /* Will this jump to test at bottom? */
    }
    } /* Otherwise there are no files: Assemble stdin (default) */
    process(filename, fp, flags);
    } while (argv[0] != NULL);

    The alternative method would be not to enter the loop if there are no
    files:
    Method 2:
    FILE *fp;
    char *filename;
    if (argv[0] == NULL) {
    process("standard input", stdin, flags)
    } else {
    while (argv[0]) {
    filename = *argv++;
    fp = fopen(filename, "r");
    if (!fp) {
    perror(filename);
    continue;
    }
    process(filename, fp, flags);
    }
    }

    I hope my question is not considered OT. It's really a question of
    code redundancy or speed. Note that the usual method of calling the
    program is with filenames.

    -- Marty Amandil
     
    Amandil, Feb 28, 2008
    #1
    1. Advertising

  2. Amandil

    Ian Collins Guest

    Amandil wrote:

    <snip>
    >
    > I hope my question is not considered OT. It's really a question of
    > code redundancy or speed. Note that the usual method of calling the
    > program is with filenames.
    >

    In any situation involving files, any speed advantage you may gain by
    optimising such loops is miniscule compared to the time taken accessing
    the file.

    Do what ever is easiest to understand and maintain.

    --
    Ian Collins.
     
    Ian Collins, Feb 28, 2008
    #2
    1. Advertising

  3. Amandil

    CBFalconer Guest

    Amandil wrote:
    >
    > Hi, I was wondering if someone could give me some advice. This
    > question is not necessarily C-specific, but it is the language I
    > am using, and I am unsure about one point in C.
    >
    > In my program, I want to process either the stdin or the list of
    > files on the command line. I have two ways of doing this, and I'm
    > asking if there is any advantage to doing it one way or the other.


    I suggest you write a routine to process one file, paramatized:

    int foo(FILE *f, /* other params */ ) {
    ...
    return whatever;
    }

    and call it from something that opens the files. It can always
    execute "foo(stdin, ...)" or it can open a file f, check it worked
    (not NULL) and then call "foo(f, ...)". Or you can set f to stdin
    if desired, by simply "f = stdin;". When foo returns, use "if (f
    != stdin) close(f)".

    --
    [mail]: Chuck F (cbfalconer at maineline dot net)
    [page]: <http://cbfalconer.home.att.net>
    Try the download section.



    --
    Posted via a free Usenet account from http://www.teranews.com
     
    CBFalconer, Feb 28, 2008
    #3
  4. Amandil

    Micah Cowan Guest

    Amandil wrote:
    > Hi, I was wondering if someone could give me some advice. This
    > question is not necessarily C-specific, but it is the language I am
    > using, and I am unsure about one point in C.
    >
    > In my program, I want to process either the stdin or the list of files
    > on the command line. I have two ways of doing this, and I'm asking if
    > there is any advantage to doing it one way or the other.
    >
    > Method 1: (using C pseudo-code)
    > FILE *fp = stdin;
    > char *filename = "standard input";
    > do {
    > if (argv[0] != NULL) {
    > filename = *argv++;
    > fp = fopen(filename, "r");
    > if (!fp) {
    > perror(filename);
    > continue; /* Will this jump to test at bottom? */
    > }
    > } /* Otherwise there are no files: Assemble stdin (default) */
    > process(filename, fp, flags);
    > } while (argv[0] != NULL);
    >
    > The alternative method would be not to enter the loop if there are no
    > files:
    > Method 2:
    > FILE *fp;
    > char *filename;
    > if (argv[0] == NULL) {
    > process("standard input", stdin, flags)
    > } else {
    > while (argv[0]) {
    > filename = *argv++;
    > fp = fopen(filename, "r");
    > if (!fp) {
    > perror(filename);
    > continue;
    > }
    > process(filename, fp, flags);
    > }
    > }
    >
    > I hope my question is not considered OT. It's really a question of
    > code redundancy or speed. Note that the usual method of calling the
    > program is with filenames.


    I don't consider two calls to the same function, with different
    arguments, to be particularly redundant.

    Evaluating the same test twice for all but the first iteration of the
    loop seems a little clunky to me, though. But I agree with Ian that your
    primary concern should be legibility. They seem about equivalent on that
    scale, though.

    BTW, note that argv[0], if not NULL, will indicate the name your program
    was invoked with, and not a filename argument. So you'd want to
    preincrement argv first. Also, it makes a little more sense to me, when
    you're iterating over the args list with argv as opposed to an integer
    index, to use *argv instead of argv[0]. But that's just a preference.

    I'd also go with const char* instead of char*, since you don't plan on
    changing the contents of the strings (and, in fact, should not, in the
    case of the string literal).

    --
    Micah J. Cowan
    Programmer, musician, typesetting enthusiast, gamer...
    http://micah.cowan.name/
     
    Micah Cowan, Feb 28, 2008
    #4
  5. Amandil

    Micah Cowan Guest

    CBFalconer wrote:
    > I suggest you write a routine to process one file, paramatized:
    >
    > int foo(FILE *f, /* other params */ ) {
    > ...
    > return whatever;
    > }
    >
    > and call it from something that opens the files.


    Um, yeah. That's what he already did! ;)

    (if it's any consolation, I skimmed it and nearly wrote your post, too.)

    --
    Micah J. Cowan
    Programmer, musician, typesetting enthusiast, gamer...
    http://micah.cowan.name/
     
    Micah Cowan, Feb 28, 2008
    #5
  6. Amandil

    Ben Pfaff Guest

    Amandil <> writes:

    > In my program, I want to process either the stdin or the list of files
    > on the command line. I have two ways of doing this, and I'm asking if
    > there is any advantage to doing it one way or the other.


    I find that your method 2 is easier to understand. There is a
    certain amount of subtlety in the code for method 1, but method 2
    makes it much more obvious what is happening. Thus, I would use
    method 2.
    --
    Ben Pfaff
    http://benpfaff.org
     
    Ben Pfaff, Feb 28, 2008
    #6
  7. Amandil

    Amandil Guest

    On Feb 28, 1:00 am, Micah Cowan <> wrote:
    > Amandil wrote:
    > > Hi, I was wondering if someone could give me some advice. This
    > > question is not necessarily C-specific, but it is the language I am
    > > using, and I am unsure about one point in C.

    >
    > > In my program, I want to process either the stdin or the list of files
    > > on the command line. I have two ways of doing this, and I'm asking if
    > > there is any advantage to doing it one way or the other.

    >
    > > Method 1:

    <snippd for brevity>
    > > The alternative method would be not to enter the loop if there are no
    > > files:
    > > Method 2:

    <snipped for brevity>
    > > I hope my question is not considered OT. It's really a question of
    > > code redundancy or speed. Note that the usual method of calling the
    > > program is with filenames.

    >
    > I don't consider two calls to the same function, with different
    > arguments, to be particularly redundant.
    >
    > Evaluating the same test twice for all but the first iteration of the
    > loop seems a little clunky to me, though. But I agree with Ian that your
    > primary concern should be legibility. They seem about equivalent on that
    > scale, though.


    Then for your first point, method 2 seems better.

    > BTW, note that argv[0], if not NULL, will indicate the name your program
    > was invoked with, and not a filename argument. So you'd want to
    > preincrement argv first. Also, it makes a little more sense to me, when
    > you're iterating over the args list with argv as opposed to an integer
    > index, to use *argv instead of argv[0]. But that's just a preference.
    >


    As you may have noticed (in your post to CBF), this is the code that
    is calling the process() function to do the work. One of the options
    is 'flags'; in other words, I have by this point already processed the
    command line somewhat (using the getopt()-like code I took from K&R2,
    mentioned in a previous post). As such, argv now points to the first
    non-option argument. Yes, I know, I should have made it clear, but
    that wasn't the point.

    > I'd also go with const char* instead of char*, since you don't plan on
    > changing the contents of the strings (and, in fact, should not, in the
    > case of the string literal).


    I didn't use const char* because I possibly do want to change the
    string that filename points to (such as changing the file extension,
    which doesn't change the length), though if I do that I'd have to
    change the way filename is set up, possibly to
    char filename[] = "standard input" and then pass filename instead
    of a string.
    Either way, filename is not intended to be const, I therefore left off
    the const keyword (which in any case is not needed if I program
    carefully enough ;-).

    BTW, I just wanted to make sure that in my do loop, the continue will
    jump to the test at the bottom before going on to the next iteration,
    right?

    Thanks for your help.

    -- Marty Amandil
     
    Amandil, Feb 28, 2008
    #7
  8. Amandil

    Micah Cowan Guest

    Amandil wrote:
    > BTW, I just wanted to make sure that in my do loop, the continue will
    > jump to the test at the bottom before going on to the next iteration,
    > right?


    Yup.

    --
    Micah J. Cowan
    Programmer, musician, typesetting enthusiast, gamer...
    http://micah.cowan.name/
     
    Micah Cowan, Feb 28, 2008
    #8
  9. Amandil

    CBFalconer Guest

    Amandil wrote:
    > Micah Cowan <> wrote:
    >

    .... snip ...
    >
    >> I'd also go with const char* instead of char*, since you don't
    >> plan on changing the contents of the strings (and, in fact,
    >> should not, in the case of the string literal).

    >
    > I didn't use const char* because I possibly do want to change
    > the string that filename points to (such as changing the file
    > extension, which doesn't change the length), though if I do that
    > I'd have to change the way filename is set up, possibly to
    > char filename[] = "standard input"
    > and then pass filename instead of a string.


    However, you had, I believe:

    char *filename = "something";

    and that SHOULD be a const char. Whether or not you put in the
    const, the string is non-alterable. If you define it as const you
    will get an error at compile time. Without that, you will get an
    error at run time.

    --
    [mail]: Chuck F (cbfalconer at maineline dot net)
    [page]: <http://cbfalconer.home.att.net>
    Try the download section.



    --
    Posted via a free Usenet account from http://www.teranews.com
     
    CBFalconer, Feb 28, 2008
    #9
  10. Amandil

    Amandil Guest

    On Feb 28, 1:23 pm, CBFalconer <> wrote:
    > Amandil wrote:
    > > Micah Cowan <> wrote:

    >
    > ... snip ...
    >
    > >> I'd also go with const char* instead of char*, since you don't
    > >> plan on changing the contents of the strings (and, in fact,
    > >> should not, in the case of the string literal).

    >
    > > I didn't use const char* because I possibly do want to change
    > > the string that filename points to (such as changing the file
    > > extension, which doesn't change the length), though if I do that
    > > I'd have to change the way filename is set up, possibly to
    > > char filename[] = "standard input"
    > > and then pass filename instead of a string.

    >
    > However, you had, I believe:
    >
    > char *filename = "something";
    >
    > and that SHOULD be a const char. Whether or not you put in the
    > const, the string is non-alterable. If you define it as const you
    > will get an error at compile time. Without that, you will get an
    > error at run time.


    You mean I might get an error at runtime. However, within process(), I
    change filename to point at a different string, one that is alterable.
    Thus, within process(), filename is not a const char *, and there is
    no reason to declare it otherwise externally, even though at one point
    I send it a const char *. The test (within process()) to see if the
    string passed was a const char * is simple: if (fp == stdin): as this
    is the only case.

    We may have to agree to disagree (or insist on arguing, as you wish)
    on this point.

    -- Marty Amandil
     
    Amandil, Feb 29, 2008
    #10
  11. On Wed, 27 Feb 2008 22:53:59 -0800, Ben Pfaff <>
    wrote:

    > Amandil <> writes:
    >
    > > In my program, I want to process either the stdin or the list of files
    > > on the command line. I have two ways of doing this, and I'm asking if
    > > there is any advantage to doing it one way or the other.

    >
    > I find that your method 2 is easier to understand. There is a
    > certain amount of subtlety in the code for method 1, but method 2
    > makes it much more obvious what is happening. Thus, I would use
    > method 2.


    Concur. And add:

    <disclaimer> This is only style, not any kind of language requirement.
    But the OP did ask a style question.

    For a pointer moving over an array, I would use only *argv and *argv++
    etc., not argv[0]. Subscript notation even [0] makes me think at least
    at first I have a fixed pointer (in)to an array, or an actual array.
    Even more, I would probably name it differently, maybe argp.

    - formerly david.thompson1 || achar(64) || worldnet.att.net
     
    David Thompson, Mar 10, 2008
    #11
    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. Replies:
    0
    Views:
    382
  2. Fano
    Replies:
    0
    Views:
    1,026
  3. Jay
    Replies:
    3
    Views:
    537
    Govindan
    Sep 9, 2003
  4. Inyeol
    Replies:
    7
    Views:
    401
    DevPlayer
    Jan 13, 2011
  5. Trans
    Replies:
    18
    Views:
    176
    Trans
    Mar 12, 2008
Loading...

Share This Page