Style and Decompostion (was Re: help needed with filnames as commandline arguments)
Simon Biber wrote (minisculy edited):[color=blue]
>[/color]
.... snip ...[color=blue]
>
> #include <stdio.h>
> int main(int argc, char **argv)
> {
> int i;
> for (i = 1; i < argc; i++) {
> int value = 0;
> FILE *fp = fopen(argv[i], "r");
> if (fp) {
> if (fscanf(fp, "%d", &value) == 1)) {
> printf("%s: %d\n", argv[i], value);
> }
> fclose(fp);
> }
> }
> return 0;
> }[/color]
While C has, for some time, allowed this sort of declaration
within a block, I see very little use for it. The code is much
more flexible if broken into a separate routine. The only cost is
(possibly) the time to call and return, which would be vanishingly
small here compared with the actual routine action. Thus I would
recommend:
#include <stdio.h>
static void tryfile(const char *fn)
{
int value;
FILE *fp;
if (fp = fopen(fn, "r"))
if (fscanf(fp, "%d", &value) == 1) {
printf("%s: %d\n", fn, value);
}
fclose(fp);
}
}
int main(int argc, char **argv)
{
int i;
for (i = 1; i < argc; i++) {
tryfile(argv[i]);
}
return 0;
}
which, at least to me, is clearer in that each routine has one
simple responsibility. With C99 you can also have all the
benefits of the single routine method by using the inline
directive. YMMV.
With this organization it is easier (to me) to see that one should
worry about the action of fopen on an empty string, and trivial to
avoid it. We can also easily see that initialization during
declaration is not needed anywhere.
There is no need to agree with me :-) but at least you can
evaluate my preference after seeing the reasons.
--
Chuck F (cbfalconer@yah oo.com) (cbfalconer@wor ldnet.att.net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home .att.net> USE worldnet address!
Simon Biber wrote (minisculy edited):[color=blue]
>[/color]
.... snip ...[color=blue]
>
> #include <stdio.h>
> int main(int argc, char **argv)
> {
> int i;
> for (i = 1; i < argc; i++) {
> int value = 0;
> FILE *fp = fopen(argv[i], "r");
> if (fp) {
> if (fscanf(fp, "%d", &value) == 1)) {
> printf("%s: %d\n", argv[i], value);
> }
> fclose(fp);
> }
> }
> return 0;
> }[/color]
While C has, for some time, allowed this sort of declaration
within a block, I see very little use for it. The code is much
more flexible if broken into a separate routine. The only cost is
(possibly) the time to call and return, which would be vanishingly
small here compared with the actual routine action. Thus I would
recommend:
#include <stdio.h>
static void tryfile(const char *fn)
{
int value;
FILE *fp;
if (fp = fopen(fn, "r"))
if (fscanf(fp, "%d", &value) == 1) {
printf("%s: %d\n", fn, value);
}
fclose(fp);
}
}
int main(int argc, char **argv)
{
int i;
for (i = 1; i < argc; i++) {
tryfile(argv[i]);
}
return 0;
}
which, at least to me, is clearer in that each routine has one
simple responsibility. With C99 you can also have all the
benefits of the single routine method by using the inline
directive. YMMV.
With this organization it is easier (to me) to see that one should
worry about the action of fopen on an empty string, and trivial to
avoid it. We can also easily see that initialization during
declaration is not needed anywhere.
There is no need to agree with me :-) but at least you can
evaluate my preference after seeing the reasons.
--
Chuck F (cbfalconer@yah oo.com) (cbfalconer@wor ldnet.att.net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home .att.net> USE worldnet address!
Comment