Which programming method is better?

A

Amandil

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
 
I

Ian Collins

Amandil wrote:

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

CBFalconer

Amandil said:
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)".
 
M

Micah Cowan

Amandil said:
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).
 
M

Micah Cowan

CBFalconer said:
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.)
 
B

Ben Pfaff

Amandil said:
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.
 
A

Amandil

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
 
M

Micah Cowan

Amandil said:
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.
 
C

CBFalconer

Amandil said:
.... 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.
 
A

Amandil

Amandil said:
Micah Cowan <[email protected]> wrote:

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

David Thompson

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
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top