[H-GEN] C code.

[Anthony Towns]

-----BEGIN PGP SIGNED MESSAGE-----


On Mon, 27 Oct 1997, The fuzzy one wrote:
> On Mon, 27 Oct 1997, Andrae Muys wrote:
> ;Anyone who's interested can have a look at
> ;http://www.uq.edu.au/~cmamuys/source_downloads/hash.[ch] constructive
> ;critism appreciated.
> "It is a convention and courtesy to keep to 80 columns per line"
> 	A little more serious though, I find the code difficult to read
> due to bunching on the right hand side.

Heh.

Reading in a nice GUI with a 1024x768 or better resolution on a 17"
(or better) monitor is the _only_ way to code.

~50 lines on screen at once, with just short of two 80 column pages
side by side.

Wonderful.

Beyond wonderful.

Absolutely _amazing_.

> There is a serious lack of vertical whitespace,

Somewhat agreed. The only places I really noticed it is in the
allocation function and the test harness. Elsewhere the indenting
works fine for me.

> Oh.  You've got it in the header file as an extern char.  That
> comes close to making sense, but the only reason you would do it is to
> prevent having to rewrite header files... 

No, the real reason is to avoid duplicating the array's contents in
every place that includes the header file.

> <counfused by uint>  Not defined anyhere must be standard C that I
> haven't seen before.

It's not. It's POSIX. It's in <sys/types.h>. It's used in a lot of
places, though: BeOS headers have similar things, and the Amiga
headers did too (<exec/types.h>, IIRC).

> 	Ahh, I should have read the comments in the header file before
> devling into the C. 

Like _duh_.

``Ahh, I should have read the documentation before delving into the
C.''

_Sheesh_!

> The EXIST and NEXIST are success results... So one
> function returns ERROR/NEXIST/EXIST, and another returns ERROR/OK.  I
> still don't like it though.  I would seperate them into two types of
> return value (both integer naturally). 

Both enumeration types, you mean, surely.

I probably wouldn't, although I'd keep telling myself that I really
_should_. Maguire recommends this in _Writing Solid Code_ very
strongly.

> I can see that you want to avoid straight
> boolean because of the common confusion as to when a return value should
> be true, and when false, but this amalgamation is... but then I'm picky.

Bah. "if ( AddValueToTable(...) )" reads perfectly unambiguously.

true/false returns are ambiguous only where they shouldn't have been
used in the first place. Returning whether something existed already
or not is an excellent example.

Hmmm.

Another way to do that might be:

bool AddValueToTable( Table *t, void* v, void** oldv );

where oldv is NULL if there was no old element and points at the old
element if there was. It still mixes return values, but possibly does
it mildly better that way.
 
> 	A bit ugly having to alloc twice during init, but no real way
> around it... not unless you want to got the dirent way and put an array[1]
> at the end, then just malloc what you need <cringe>

Yeah. That's also not entirely legal C, either.
 
> Oh, and I don't like ==.  One
> of my phobias coming from numerical maths.  I know and you know that your
> code is written using unsigned in,

I'm wondering actually that that shouldn't be a "size_t".

In any case, having a negative number is beyond meaningless (it has
negative meaning?) so if I, personally, found someone trying to
maintain that piece of code who didn't realize what was going on
I'd... Well. I'd ask Bruce for advice. Put it that way.

> You're only undefining it to define it again with comments around
> it.  I hope none of those strings have close comment characters in them
> <evil grin>.  

Comments are removed before macros are substituted in.

So you'll compile happily with HASH_DEBUG undefined, and you'll get a
warning about the */ if it is defined. 

> I'd personally make it another function.  That way it can
> accept non-fixed strings, doesn't have to comment the code out, and
> doesn't need a complex definition.  

And it requires a function call that just plain isn't necessary.

C isn't a hard language. It's not unreasonable to expect people who
code in it to actually _know_ it, IMAO.

Certain lecturers not withstanding.

And yes, Ada _is_ a different story. So's C++, IMO.

> It can include an ifdef HASH_DEBUG
> around the code within it, and I would probably give the compiler a pragma
> to inline it, just to be sure it didn't cost a performance hit on release
> day.

You can't inline C functions.

You can with gcc, and you can in C++, but you can't in C.

At least, not until C9X comes out.
 
> Hmm, I'd personally have the old value returned from
> intHashAddEntry. You'd avoid a *.  *s are bad.  

*shrug*

It might be better to have typedef both IntHashTable* and void* since
you never actually dereference either in the interface parts.

If you wanted to mix in your status and result, this would even get
rid of your void***:
	objptr *intHashDel( IntHashTableRef *table );
		/* returns NULL on error */

Using an IntHashTableRef rather than an IntHashTable* outright has the
small advantage that you can later change the interface part to be a
handle of some sort (thus allowing virtual memory, shuffling if your
heap gets overly fragmented and so on).

Cheers,
aj

- --
Anthony Towns <aj at humbug.org.au> <http://student.uq.edu.au/~s343676/>
I don't speak for anyone save myself. PGP encrypted mail preferred.

``Do. Not. Taunt. The. Happy. Fun. Novak.
	It will get gruesome, quickly.'' -- John Dilick, rasfwr-j

-----BEGIN PGP SIGNATURE-----
Version: 2.6.3ia
Charset: ascii
Comment: Key available at http://student.uq.edu.au/~s343676/aj_key.asc

iQCVAwUBNFSuMuRRvX9xctrtAQFXYAP/a/JDGlx4tUDtNMBD2wYGCYOD+8nYpGFd
oSZ7+SAmcrd3i0pEle7OAyhWIXBR6gsSJehIArXWTDWSjt79Yq29x79BUMhlA9Hj
Z6rxukUY6+qBn8WYWcjZk/FQYCeXaRGLUA6sBRatkZTfM+JPovb0pvR5UfhUghbv
ZuZASDXS61o=
=AzTk
-----END PGP SIGNATURE-----

----------------------- HUMBUG General List --------------------------------
echo "unsubscribe general" | mail majordomo at humbug.org.au # To Unsubscribe