Post by James R. KuyperPost by John ForkoshPost by Malcolm McLeanPost by John ForkoshI have two large modules, let's say this.c and that.c, that
unexpectedly need to be compiled together,
cc this.c that.c -o thisandthat
Everything would work okay (just one main() function, etc),
except that each module contains several dozen functions with
name collisions with the other module, e.g., new_raster() and
delete_raster() in both modules, but the raster structs are a bit
different in this.c and that.c, so the functions are different.
...
Post by John ForkoshPost by Malcolm McLeanIt should take only minutes to add the keyword "static" to all your non-
exported functions.
If "only minutes", I wouldn't have even bothered writing the post.:)
~10 hours and counting (maybe ~half done). 156 functions in 18K lines
of code. ...
I wasn't the one who suggested that it would "take only minutes", but I
found that estimate entirely reasonable. You said there were only
"several dozen functions with name collisions". It shouldn't take very
long to add even several dozen "static" keywords to a program. Now
you're implying that most of the 156 functions needed modification.
Having that many identically named functions between two different
modules implies a huge amount of functional overlap between the modules.
I would suspect that much of the code in the second module was created
by copying it from the first, and then making modifications. If so, I
suspect you may have missed an opportunity for code re-use: many of the
identically named functions could probably, with a little re-design,
have been not merely identically named, but actually the same function.
If necessary, you could have made them behave differently when called
for the two different purposes, based upon the value of a parameter.
Yeah, very often copied-and-modified. But "based on value of parameter"
assumes I knew what I wanted from the very beginning. Many are functions
developed over years (and years). For example,
/* ======================================================================
* Function: strpspn ( char *s, char *reject, char *segment )
* Purpose: finds the initial segment of s containing no chars
* in reject that are outside (), [] and {} parens, e.g.,
* strpspn("abc(---)def+++","+-",segment) returns
* segment="abc(---)def" and a pointer to the first + in s
* because the -'s are enclosed in () parens.
* ----------------------------------------------------------------------
* etc */
essentially spans ()parens, but initially forgot about internal quotes
like (abc")))"def) which should span the ")))", too. Didn't occur to me
for first version. In this case, wouldn't hurt if all programs using
it got that fix. But many function changes were more subtle, and couldn't
be transparently backwards applied to all programs using earlier versions.
Post by James R. KuyperPost by John Forkosh... And big problem isn't adding static (actually, my #define'd
symbol FUNCSCOPE), but commenting out declarations within calling
functions, which typically explicitly declare each called function, e.g.,
Well, that was a bad idea, and this is one of the reasons.
Post by John Forkoshraster /**new_raster(),*/ *rp=NULL; /*image raster returned to caller*/
/*int delete_raster();*/ /*in case rasterization fails*/
Post by Malcolm McLeanIt's something you should have done anyway and as a matter of course.
In retrospect, yeah. But I've seen lots and lots of code (maybe >75% of
the stuff I look at) without "static" where it should "pedantically"
have been. Certainly most of mine. And are you throwing stones from
inside a glass house?:)
A very large fraction of my code was originally designed and written by
other people, and has only one function per module, so opportunities for
declaring functions static don't come up very often.
My modules tend to get ginormous becuase they're typically gpl'ed
with a mostly end-user target audience not entirely comfortable
with building from source. So I don't want to give them distributions
containing hundreds of small files, which just gets intimidating to
people unfamiliar with configure/make/make_install (or similar).
A few large files results in much less email from confused end-users.
And for people actually reading them, I have extensive comments,
including a table of contents within a large top-of-file comment
block, of the form,
* Functions: o The following "table of contents" lists each function
* comprising gifscroll in the order it appears in this file.
* See individual function entry points for specific comments
* about purpose, calling sequence, side effects, etc.
* =============================================================
* +---
* | gifscroll functions
* +-----------------------
* main(argc,argv) cgi driver for gifscroll
* rasterize_this(msgorpbm,type) rasterize message or pbm file
* new_raster(width,height) raster allocation and constructor
* delete_raster(rp) destructor for raster
* rastcpy(rp,width,height,isfree) duplicate copy of rp
* rastput(target,source,tupleft,supleft,bg,isopaque) put s on t
* boxcols(rp,bp,irow,col1) column pixels of bp->shape at irow
So these kinds of extensive comments hopefully make the large files
more manageable. At least it hasn't become unmanageable or unreadable
for me, and no particular size-specific problems until this pesky
static thing.
Post by James R. KuyperHowever, whenever
I've been free to design new code or make a major change to my existing
code, I usually define multiple functions in each module, but usually
only one of them has external linkage. So no, I'm not "throwing stones
from inside a glass house".
Well, had it occurred to me I'd ever want to compile these modules
together, I'd have been more careful from the beginning.
So, if not "...from a glass house", then you (and several others)
are at least telling me I should have closed the barn door >>after<<
the horse has bolted. Believe me, as soon as I saw the horse bolting,
I immediately realized, all by myself, I should've closed the door.
Original question was asking for an easy way to re-corral the horse.
Post by James R. Kuyper...
Post by John ForkoshYeah, a dozen or two of the namespace collision functions are indeed
for string_handling/expression_parsing. Mostly just duplicates in each
module, but sometimes "newer and improved'er". Never figured out how
to deal with evolving libraries. If kept in a separate module, any
changes may affect all programs using it, involving elaborate maintenance.
So library either tends to get frozen, or you have many different versions
floating around. ...
A library function that is shared between multiple different programs
should be changed only if the change is one that you want to occur in
all of those programs. If that's not the case, freezing the code is
entirely reasonable, and I don't see that as a problem: it's a normal
feature of any sufficiently generic function, like memcpy().
Nor do I see it as a problem to have multiple versions of a routine if
those versions need to do different things - but I'd recommend giving
them different names that reflect those different things that they're
doing. Again, this is perfectly normal feature of the kinds of functions
that inexperienced programmers tend to give names like "process_data()"
or "make_object()".
It really sounds like your functional decomposition of your code is poor.
That's a whole other issue. And I've occasionally considered refactoring,
but it ain't worth the effort. Functional decomposition >>becomes<<
poor over time, as "bags-on-bags" development, necessitated by new
and unanticipated functional requirements, screws up what may have
been an originally reasonable design. You sound like people without
much real-world experience sound, who think you can write a
functional requirements document, and then detail design and code
from that, once and for all. But nothing ever stays unchanged.
Everything's best viewed as a prototype for its next version.
The most productive development style is "bags-on-bags" until the
whole thing becomes such a mess that you have to pretty much
chuck it and start from scratch, but hopefully with a better
understanding. Refactoring every time there's the slightest
possible improvement is almost always a big waste of time and money.
I'd rather add new features, visible to end users, to messy old code,
than try to clean it up with no other purpose. Only when it
gets to the point of unmaintainability is refactoring a justifiable
purpose in and of itself.
Post by James R. KuyperPost by John Forkosh... Better to just put a copy (current version) in each
module using it, whereby it may eventually become somewhat stale, but
Well, that would tend to explain why your modules are so ridiculously
over-sized.
Well, that's your opinion which you're entitled to.
I'd personally disagree with most of what you're saying,
at least disagree with it as hard-and-fast rules.
Different situations call for different strategies.
For example, when working with a large (or even small) team,
I'd obviously never consider such large modules.
But when working solo, I consider what I can best handle.
And I also feel that I'm a better judge of that than you.
Now, you might maybe want to pick up my code, and then
maybe consider it over-sized for your working habits.
Conversely, I might pick up yours and consider it under-sized.
But I'd acknowledge that as a personal preference rather
than a universal truth.
--
John Forkosh ( mailto: ***@f.com where j=john and f=forkosh )