[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [coldsync-hackers] speed



On Monday 20 October 2003 08:11, Kevin Velghe wrote:
> >> This is great! I'll try to write a clean conduit that may be used as a
> >> starting point to write more of these in C.
> >
> > Yes, please. I'd hate to see my little hacks become a standard of some
> > kind.
> > They don't use getopt(), the header handling is horrible (and,
> > actually,
> > has some blatant bugs), and I deliberatly didn't work with records more
> > complicated than memo because I didn't want to get into application
> > record
> > structures, endianness, and all that stuff.
>
> The header handling : I'm not that used to writing super-clean code
> (it's not a mess though...) and I guess that's why I can't really see
> the problem with the header handling. It seems quite fast, and it reads
> everything into arrays. The only thing I would improve here is to split
> the name and the data of the header into a 2 dimensional array. What
> should it be able to do according to you so I can make a clearer
> picture in my mind of what "clean" code is? :-)

Looking at the examples again, they're actually not that awful. A little 
polish and bug fixing and I probably wouldn't feel quite so bad about them. 
My main concern is really that there's a whole lot 'o code doing trivial 
crap like checking the command line and reading the headers that just 
dwarfs the meat of the conduit; accessing databases and records and such. 
I'm never comfortable when the real work is buried under a pile of 
administration. Maybe I've just been working for the government for too 
long. Anyhow, the gist of what I'm saying is that all that command line and 
header stuff should really be relegated to a ColdSync API so you can just do 
stuff like:

int main( int argc, char* argv[] ) {
	PConnection* pconn;
	Conduit* conduit = csInitConduit( argc, argv, stdin );
	if( csHeader(conduit,"SPCpipe") == NULL ) {
		fprintf( stderr, "%s: No SPC pipe\n", argv[0] );
		exit( -1 );
	}
	pconn = new_spc_client( atoi(csHeader(conduit,"SPCpipe")) );

	/* do conduit stuff */
}

But that's not how it's done right now, so back to reality.

Specific things that bug me right now:

Command-line parsing in Unix code should pretty much always use getopt(). I 
don't write much command-line apps so I didn't bother.

Aside from silly bugs like:
                        headers[nheaders++] = strdup( headers );
(should be strdup(tmp)), the header handling is simple. You actually 
wouldn't gain much by breaking it into separate arrays.

One (mostly philosophical) concern is that you rarely access the headers as 
an array. It's almost always Dictionary access patterns. So the current 
scheme, while it works, isn't terribly optimal. A C++ implementation using 
a dictionary class (STL or whatever) would be much cleaner.

In C, a linked list would be easiest if it was wrapped in a proper API. When 
I was writing the examples, I seriously thought of storing headers using 
putenv() and using getenv() to extract values. Yuck. Bad hacker, bad! But 
something like that would make like cleaner.

Preference handling is only halfway done. It still needs to read the binary 
data after the blank line and it seriously needs a clean interface for 
reading the actual pref data.

c.
This message was sent through the coldsync-hackers mailing list.  To remove
yourself from this mailing list, send a message to majordomo@thedotin.net
with the words "unsubscribe coldsync-hackers" in the message body.  For more
information on Coldsync, send mail to coldsync-hackers-owner@thedotin.net.