Critique

Wow, has it ever been a long time since I posted anything to this
group. I don't keep as current as I used to anymore; I only lurk from
time to time when I have a spare moment.
I am trying to write a program to open a file and process it
according to command line parameters. All I need are two simple search
and replace operations. I invoke the program with an int representing a
site code and a long representing a date and my program should replace
some hard-coded text strings in the file with the int and the long I pass
in. I may have some insight as to why this is not working now, but I'd
like some constructive criticism here. I tried to conform to the standard
(as I know it), but, well, let's see how it goes... Thanks for reading!

Advertisements

Andrew Clark wrote:
>
> Wow, has it ever been a long time since I posted anything to this
> group. I don't keep as current as I used to anymore; I only lurk
> from time to time when I have a spare moment.
>
> I am trying to write a program to open a file and process it
> according to command line parameters. All I need are two simple
> search and replace operations. I invoke the program with an int
> representing a site code and a long representing a date and my
> program should replace some hard-coded text strings in the file
> with the int and the long I pass in. I may have some insight as
> to why this is not working now, but I'd like some constructive
> criticism here. I tried to conform to the standard (as I know
> it), but, well, let's see how it goes... Thanks for reading!

Fundamentally you try to cram too much into one routine, main.
Things will be much clearer if you break operations up. Your
getline is a step in the right direction.

However there is available code to do almost exactly what you
want. Take a look on my site for ggets and id2id-20. id2id will
do your job if you write a two line configuration file to supply
the replacements for xxx and yyyyyyyy, but this may not be quite
suitable. At any rate the portable source is there, and you can
easily change it to work from command line parameters. See:

<http://cbfalconer.home.att.net/download/>

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson

/* subtract 1 for "site" -> "xxx" and */
/* add one for NUL character */
outfile = malloc (strlen (IN_) - 1 + 1);
/* -ed-
* the requested size should eventually
* be stored somewhere for further controls...
*
* That said, unless you intent to realloc the array,
malloc()
* is not necessary here because the size is a compile-time
* constant if expressed by 'sizeof IN - 1', IN being a
string
* literal.
*
* Of course, if it becomes a variable (array of string),
it's
* a different story.
*
* An array would suffice.
*/

while (EOF != fgetline (in, buf))
/* -ed- BAD! You have reinvented the gets() bug.
* You have missed the opportunity to build a
solid
* read line function with flexible bound
checking.

while (EOF != fgetline (in, buf, sizeof buf))

*/

{
if (buf == strstr (buf, "--"))
{
/* If the line is a comment, do not */
/* include it in the final script */
continue;
}
else if ((p = strstr (buf, "xxx")) != NULL)
{
/* -ed-
* why is the definiton of 'temp' not here ?
* Scope reduction makes the code more
readable,
* more safe, and prepare it to modularization.
*/

Does anyone agree with this notion? Having separate prototypes at the top of
a file relieves me of the burden of having to arrange my function
implementations in a specific order; more specifically, it allows me to
always have main() as the first function. For that, I am willing to pay a
price in the form of a small, innocuous redundancy.

Christian Kandeler wrote:
> Emmanuel Delahaye wrote:
>
>> int fgetline (FILE *, char *);
>> /* -ed- avoid separated prototypes with a better layout... */
>
> Does anyone agree with this notion? Having separate prototypes at
> the top of a file relieves me of the burden of having to arrange my
> function implementations in a specific order; more specifically, it
> allows me to always have main() as the first function. For that, I
> am willing to pay a price in the form of a small, innocuous
> redundancy.

Definitely. That redundance can be the source of silly hard to
find errors. By paying strict attention to "declare and define
before use" I always know which way to look for the routine
source. The only (and rare) exception is when designing mutually
recursive functions.

Since we are no longer using line oriented editors there is no
penalty to be paid for writing main first, and then moving above it
to create the various subroutines. By creating them as stubs we
can check a good deal of our logic very early.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson

On Tue, 12 Apr 2005 09:08:24 +0200, Christian Kandeler wrote:
> Emmanuel Delahaye wrote:
>
>> int fgetline (FILE *, char *);
>> /* -ed- avoid separated prototypes with a better layout... */
>
> Does anyone agree with this notion? Having separate prototypes at the top of
> a file relieves me of the burden of having to arrange my function
> implementations in a specific order; more specifically, it allows me to
> always have main() as the first function. For that, I am willing to pay a
> price in the form of a small, innocuous redundancy.

It is a matter of personal style and taste. Personally I agree with you.
Having higher level functions first is a natural layout for a top-down
divide and conquer approach to programming. It is entirely reasonable that
the first function you come to when you look at a program is where the
program starts. That's also true for "modules" where you encounter the
(external linkage) functions that defvine the interface first and any
helper functions (internal linkage) they need come after.

Some people don't like separate declarations but

1. you need them anyway (in a header) for functions with external linkage
so the issue here applies only to internal linkage functions.

2. The declarations provided a VALIDATED summary of the functions in the
source file which can be amazingly useful. The validation also means
that there is very little chance of a functionality impacting error in
the declarations going undiagnosed.

In short, in my view top-down is the natural way to organise a program and
doing things the natural way can only help readability. The perceived
downside of this appraoch i.e the extra lines you have to type and the
possibility of inconsistency turn out to be rather insignificant when you
actually look into it, but the readability advantages, especially the
valiated summary are far from insignificant. When you are looking at a
source file for the fist time having such a summary is a great help. I
would argue that you should provide a function declaration list even if
your preferred style is "backwards".

The bottom line is however that you should do what is most natural to you,
unless you are working to a project style in which case it is more
important to be consistent.

Andrew Clark wrote:
> I am trying to write a program to open a file and process it
> according to command line parameters. All I need are two simple
> search and replace operations. I invoke the program with an int
> representing a site code and a long representing a date and my
> program should replace some hard-coded text strings in the file
> with the int and the long I pass in. ...

Why use a utility routine that strips the newline, only to
put it back manually each time?
> <snip>

You handle comments inconsistently in that a '-- xxx' in the
middle of a line may invoke substitution, but an -- comment
at the very start of a line won't. Also, you ignore /* */
style comments.

I think you can simplify the whole task by scanning character
by character with a simple state machine (ignoring sql comments
as you go). All you need to do is check is whether an 'x' is
followed by two more 'x's, and whether a 'y' is followed by 7
more 'y's. If an 'x' isn't followed by two more, then just
output the number of consecutive 'x's read to that point.

Going off topic, I'm sure you can find existing tools to do
such simple replacements.

Personally, I think even those tools are probably overkill
for the deeper problem at hand.

Since you already have a template file, let's suppose that file
can be abstracted down to...

CBFalconer wrote:
> Christian Kandeler wrote:
> > Emmanuel Delahaye wrote:
> >
> >> int fgetline (FILE *, char *);
> >> /* -ed- avoid separated prototypes with a better layout... */
> >
> > Does anyone agree with this notion? Having separate prototypes at
> > the top of a file relieves me of the burden of having to arrange my
> > function implementations in a specific order; more specifically, it
> > allows me to always have main() as the first function. For that, I
> > am willing to pay a price in the form of a small, innocuous
> > redundancy.
>
> Definitely. That redundance can be the source of silly hard to
> find errors. By paying strict attention to "declare and define
> before use" I always know which way to look for the routine
> source. The only (and rare) exception is when designing mutually
> recursive functions.
>
> Since we are no longer using line oriented editors there is no
> penalty to be paid for writing main first, and then moving above it
> to create the various subroutines. By creating them as stubs we
> can check a good deal of our logic very early.
>

What errors are you referring to? I use the "prototypes, main,
subroutines"
layout, and I have not had any problems with it. True, if I change
the calling sequence of a subroutine, I have to update the prototype,
but if I do not, I get a compilation error.

On Tue, 12 Apr 2005, Lawrence Kirby wrote:
>
> On Tue, 12 Apr 2005 09:08:24 +0200, Christian Kandeler wrote:
>> Emmanuel Delahaye wrote:
>>> /* -ed- avoid separated prototypes with a better layout... */
>>
>> Does anyone agree with this notion? Having separate prototypes at the
>> top of a file relieves me of the burden of having to arrange my function
>> implementations in a specific order; more specifically, it allows me to
>> always have main() as the first function. For that, I am willing to pay a
>> price in the form of a small, innocuous redundancy.
>
> It is a matter of personal style and taste. Personally I agree with you.
[...]
> 2. The declarations provided a VALIDATED summary of the functions in the
> source file which can be amazingly useful. The validation also means
> that there is very little chance of a functionality impacting error in
> the declarations going undiagnosed.

True. Also, I have within the past year started writing my top-of-file
prototypes with indentation corresponding to the caller-callee
relationships between them:

This order may or may not correspond to the order of the actual function
definitions, but that's why editors have the "Find" command. I think it
is useful to have this kind of documentary stuff at the top of each file.
(Well, /near/ the top... above this I have directives, a block comment,
probably some struct and enum definitions... but basically at the top.
> The bottom line is however that you should do what is most natural to you,
> unless you are working to a project style in which case it is more
> important to be consistent.

William Hughes wrote:
> CBFalconer wrote:
>> Christian Kandeler wrote:
>>> Emmanuel Delahaye wrote:
>>>
>>>> int fgetline (FILE *, char *);
>>>> /* -ed- avoid separated prototypes with a better layout... */
>>>
>>> Does anyone agree with this notion? Having separate prototypes
>>> at the top of a file relieves me of the burden of having to
>>> arrange my function implementations in a specific order; more
>>> specifically, it allows me to always have main() as the first
>>> function. For that, I am willing to pay a price in the form of
>>> a small, innocuous redundancy.
>>
>> Definitely. That redundance can be the source of silly hard to
>> find errors. By paying strict attention to "declare and define
>> before use" I always know which way to look for the routine
>> source. The only (and rare) exception is when designing
>> mutually recursive functions.
>>
>> Since we are no longer using line oriented editors there is no
>> penalty to be paid for writing main first, and then moving above
>> it to create the various subroutines. By creating them as stubs
>> we can check a good deal of our logic very early.
>
> What errors are you referring to? I use the "prototypes, main,
> subroutines" layout, and I have not had any problems with it.
> True, if I change the calling sequence of a subroutine, I have to
> update the prototype, but if I do not, I get a compilation error.

I think whether or not you get an error is compiler dependant.
However we normally take pains to define constants in one place,
and then use that definition throughout. Why do you suddenly want
to take an entirely different attitude to function prototypes? I
see no reason to ever keep two separate entities in sync manually.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson

On Tue, 12 Apr 2005 09:08:24 +0200, Christian Kandeler
<_invalid> wrote:
>Emmanuel Delahaye wrote:
>
>> int fgetline (FILE *, char *);
>> /* -ed- avoid separated prototypes with a better layout... */
>
>Does anyone agree with this notion? Having separate prototypes at the top of
>a file relieves me of the burden of having to arrange my function
>implementations in a specific order; more specifically, it allows me to
>always have main() as the first function. For that, I am willing to pay a
>price in the form of a small, innocuous redundancy.
>
And it puts the prototypes all in one place, which I like. I do not
agree with Emmanuel on this one.

On Tue, 12 Apr 2005 07:42:33 GMT, CBFalconer <>
wrote:
>Christian Kandeler wrote:
>> Emmanuel Delahaye wrote:
>>
>>> int fgetline (FILE *, char *);
>>> /* -ed- avoid separated prototypes with a better layout... */
>>
>> Does anyone agree with this notion? Having separate prototypes at
>> the top of a file relieves me of the burden of having to arrange my
>> function implementations in a specific order; more specifically, it
>> allows me to always have main() as the first function. For that, I
>> am willing to pay a price in the form of a small, innocuous
>> redundancy.
>
>Definitely. That redundance can be the source of silly hard to
>find errors.
How so? My compiler happily informs me if there's any discrepancy.
>By paying strict attention to "declare and define
>before use" I always know which way to look for the routine
>source.
Me, too. It's just the opposite direction.
>The only (and rare) exception is when designing mutually
>recursive functions.
>
>Since we are no longer using line oriented editors there is no
>penalty to be paid for writing main first, and then moving above it
>to create the various subroutines. By creating them as stubs we
>can check a good deal of our logic very early.

CBFalconer wrote:
> William Hughes wrote:
> > CBFalconer wrote:
> >> Christian Kandeler wrote:
> >>> Emmanuel Delahaye wrote:
> >>>
> >>>> int fgetline (FILE *, char *);
> >>>> /* -ed- avoid separated prototypes with a better layout... */
> >>>
> >>> Does anyone agree with this notion? Having separate prototypes
> >>> at the top of a file relieves me of the burden of having to
> >>> arrange my function implementations in a specific order; more
> >>> specifically, it allows me to always have main() as the first
> >>> function. For that, I am willing to pay a price in the form of
> >>> a small, innocuous redundancy.
> >>
> >> Definitely. That redundance can be the source of silly hard to
> >> find errors. By paying strict attention to "declare and define
> >> before use" I always know which way to look for the routine
> >> source. The only (and rare) exception is when designing
> >> mutually recursive functions.
> >>
> >> Since we are no longer using line oriented editors there is no
> >> penalty to be paid for writing main first, and then moving above
> >> it to create the various subroutines. By creating them as stubs
> >> we can check a good deal of our logic very early.
> >
> > What errors are you referring to? I use the "prototypes, main,
> > subroutines" layout, and I have not had any problems with it.
> > True, if I change the calling sequence of a subroutine, I have to
> > update the prototype, but if I do not, I get a compilation error.
>
> I think whether or not you get an error is compiler dependant.

Probably. I use gcc for development, so maybe I am spoiled here.
However, I would expect any good compiler to pick
up on a prototype/definition error. Are there
compilers that do not catch this?

> However we normally take pains to define constants in one place,
> and then use that definition throughout. Why do you suddenly want
> to take an entirely different attitude to function prototypes?

Possibly because of the use of separate .h files for prototypes
of functions to be called from outside of the source file
(the source file includes the .h file in order to catch
prototype/definition conflicts). Possibly, because we started
doing it that way and had no problems.
>I see no reason to ever keep two separate entities in sync manually.

Well it's not much work (especially as we tend to use separate files
for each function, so prototype maintanence cannot usually be
avoided). Still, I guess I would agree with you, I just don't
consider it a very important.

Always? Very few of the C source files I create have main() in them
at all. But no doubt milage varies.
> Definitely. That redundance can be the source of silly hard to
> find errors.

How so? If the prototype does not match the definition, it's a
constraint violation. (C90 6.5: "All declarations in the same scope
that refer to the same object or function shall specify compatible
types"; by the rules of 6.5.4.3, any meaningful mismatch between a
prototype that includes a parameter list and the definition of the
corresponding function will produce incompatible function types.)
> By paying strict attention to "declare and define
> before use" I always know which way to look for the routine
> source.

Only if it's in the same TU, presumably. And is searching for a
function definition difficult?
> Since we are no longer using line oriented editors there is no
> penalty to be paid for writing main first, and then moving above it
> to create the various subroutines.

A similar argument could be made regarding editors with decent
search capabilities, code-indexing features (like ctags, "browse"
facilties, etc), and the like, and not worrying about the relative
position of various functions in a TU.
> By creating them as stubs we
> can check a good deal of our logic very early.

I fail to see how their order in the TU has any effect on this
principle.

I have no argument with "declare dependents first" as a matter of
style, but I think its purported advantages are largely subjective.

Personally, I prefer to arrange the functions in an order which seems
to me to be logical while reading the source; typically that involves
introducing higher-level functions first (and using meaningful names
for lower-level ones, plus of course ample comments), though "helper"
leaf functions that are only used in one or two places may preceed
their callers. But I stick to no hard and fast rule in this matter.
I prefer to preserve it as a degree of stylistic freedom, one of
several I try to employ to keep the code readable. Much as I might
use a dependent clause on its own as a sentence in prose.

--
Michael Wojcik

Proverbs for Paranoids, 2: The innocence of the creatures is in inverse
proportion to the immorality of the Master. -- Thomas Pynchon

FWIW I disagree. Prototyping in my experience /removes/ the ability to
have annoying errors, especially when functions are split over several
files.
>By paying strict attention to "declare and define
>before use" I always know which way to look for the routine
>source.

I personally find this a completely painful way to work. If my editor
won't tell me where to find a function, I get a better editor

I apologize for not snipping but I am really commenting on all of it.
I'm pretty sure that CBFalconer and I agree completely on this. Given a
single translation unit for your program, it makes no sense to prototype
functions before main so that you can have something like..

Alan Balmer wrote on 13/04/05 :
> I wish the programs I work with were really like that!
>
> Instead, I have something like:
>
> /* local functions */
> static void get_campus_id(void) ;
> static int read_rec(FILE *p);
<...>
>
> followed by main, then slightly under 3000 lines of code inplementing

3000 lines in a single source ? You have a design problem...
> those functions. Your way, that puts main at about the middle of page
> 49. That doesn't appeal to me.
>
The good place for main() is the last function of main.c

Easy to implement, easy to find and Vulcan-logic.
> Besides, your way it's much more difficult to print the list of
> function prototypes and hang it on the wall ;-)

If you have a good code organisation, you just have to print out the
headers where the detached prototypes are.

The 'static' functions are local and probably of lower interest. Whatis
important are the entry points.

On Wed, 13 Apr 2005 19:41:02 +0200, "Emmanuel Delahaye"
<> wrote:
>Alan Balmer wrote on 13/04/05 :
>> I wish the programs I work with were really like that!
>>
>> Instead, I have something like:
>>
>> /* local functions */
>> static void get_campus_id(void) ;
>> static int read_rec(FILE *p);
><...>
>>
>> followed by main, then slightly under 3000 lines of code inplementing
>
>3000 lines in a single source ? You have a design problem...
>
Why should I separate local function, used by no other program, into
separate source modules?
>> those functions. Your way, that puts main at about the middle of page
>> 49. That doesn't appeal to me.
>>
>The good place for main() is the last function of main.c
>
None of the main programs I work with are named main.c. And you're
just repeating what you said before. And it would still put main about
the middle of page 49.
>Easy to implement, easy to find and Vulcan-logic.
>
>> Besides, your way it's much more difficult to print the list of
>> function prototypes and hang it on the wall ;-)
>
>If you have a good code organisation, you just have to print out the
>headers where the detached prototypes are.
>
Headers? For static functions? How would putting the prototypes in a
separate header serve your goal of not having separate prototypes?
>The 'static' functions are local and probably of lower interest. Whatis
>important are the entry points.
>
There's one entry point - main(). This is a program, not a library. I
don't know why the local functions would be of "lower interest." The
program won't work without them.

<snipped another of the hundreds of "recommended layouts" I've seen in
the last 35 years - and one of the less desirable ones.>

On Tue, 12 Apr 2005 18:40:28 -0400, in comp.lang.c , Joe Wright
<> wrote:
>Mark McIntyre wrote:
>> On Tue, 12 Apr 2005 07:42:33 GMT, in comp.lang.c , CBFalconer
>> <> wrote:
>>
(of putting main at the end of the file and avoiding prototypes...)
>>>Definitely. That redundance can be the source of silly hard to
>>>find errors.
>>
>> FWIW I disagree.
>I apologize for not snipping but I am really commenting on all of it.
>I'm pretty sure that CBFalconer and I agree completely on this. Given a
>single translation unit for your program, it makes no sense to prototype
>functions before main

My problem is that this is fine for tiny programs, with a half-dozen
functions.

But as soon as you get a few lengthy functions, it becomes quite
annoying. And it makes for significant maintenance the second you need
to split the project across multiple files. So my tendency is to start
out as if the program would later become multifile. I even tend to put
all the fn prototypes into a header straight away.

Share This Page

Welcome to The Coding Forums!

Welcome to the Coding Forums, the place to chat about anything related to programming and coding languages.

Please join our friendly community by clicking the button below - it only takes a few seconds and is totally free. You'll be able to ask questions about coding or chat with the community and help others.
Sign up now!