code critique request

R

rihad

Hi there. If you remember my recent posting regarding fslurp(), well, this is
its continuation :)

A collection of useful and semi-useful file-and-line-slurping functions:
fslurp() - slurp a text file into memory and return it as an array of lines
fslurpb() - slurp a binary file into memory
fslurpl() - slurp a whole line from a text file (i.e. fgets() with no limits)

Also included are several utility functions that try to mimic the behaviour
of traditional Unix commands on the slurped text file:
fgrep(), fsort(), funiq(), freverse(), fhead(), ftail(), fcat(). And more.

Look into f.h for some documentation.

Written in ISO C.


Code is here:
http://my.baku.to/f.c
http://my.baku.to/f.h

some simple API demonstrating utilities:
http://my.baku.to/fsort.c
http://my.baku.to/funiq.c
http://my.baku.to/ftac.c
http://my.baku.to/fhead.c
http://my.baku.to/ftail.c

(it's around 1000 l.o.c. and I reckon only FAQ maintainers could post that much
or more without getting flamed. I didn't think I'd want to try :))

Please comment! Thank you very much in advance.
 
D

David Rubin

rihad said:
A collection of useful and semi-useful file-and-line-slurping functions:
fslurp() - slurp a text file into memory and return it as an array of lines
fslurpb() - slurp a binary file into memory
fslurpl() - slurp a whole line from a text file (i.e. fgets() with no limits)

You should use a better packaging scheme than just f*. Perhaps slurp_f*.
Also included are several utility functions that try to mimic the behaviour
of traditional Unix commands on the slurped text file:
fgrep(), fsort(), funiq(), freverse(), fhead(), ftail(), fcat(). And more.

These are interesting, although minimally useful in a Unix environment.
But for that matter, why would you write a text-processing application
in C on a Unix platform? Overall, the implementation seems reasonable.

The only obvious complaint is that the documentation in f.h does not
make it clear that the caller is responsible for freeing the memory
returned by the slurp functions, despite the presence of ffree.
Additionally, the inclusion of fslurpl and fslurpb seems a bit
gratuitous since you can't use the returned data with the utility
functions (in the same way that you can with the data returned by
fslurp). Thus, the library becomes "A text processing library, plus a
few random functions." I would just remove those, rename fslurp to
slurp_read, and rename everything else slurp_* (minus the 'f'). The
error indications are a bit lacking. For example, fsave does not
distinguish between a file error (fopen) or a stream error (fputs/c). It
also would be nice, from a maintenance perspective, to list the
functions in f.c in alphabetical order with the function name at the
start of the line:

char **
fforeach(...)

/david
 
R

rihad

You should use a better packaging scheme than just f*. Perhaps slurp_f*.

I like these names for their brevity and a strong C-style feel to them. But you
have a point, I shouldn't be freely polluting the f* namespace.
These are interesting, although minimally useful in a Unix environment.
But for that matter, why would you write a text-processing application
in C on a Unix platform? Overall, the implementation seems reasonable.

I actually tried to do some benchmarking, comparing them to their full-blown
counterparts from the Gnu textutils fame. While most of the functions hopelessly
lagged behind and were up to several times slower (since they read a whole file
into memory prior to processing), fsort() and funiq() were actually a bit faster
:)

Seriously though, this is all just for the fun of it. I like trying to write
code that I like much more than writing any code for production. Also a great
opportunity to do refactoring with no deadlines creeping up on you (I love
refactoring!).
The only obvious complaint is that the documentation in f.h does not
make it clear that the caller is responsible for freeing the memory
returned by the slurp functions, despite the presence of ffree.

Well, it's quite obvious that the memory is returned dynamically allocated. And
a few lines later comes ffree(). I'll just add a note to better not forget to
use ffree() when done.
Additionally, the inclusion of fslurpl and fslurpb seems a bit
gratuitous since you can't use the returned data with the utility
functions (in the same way that you can with the data returned by
fslurp).

You know, I was thinking about this and the more I thought the more I felt like
C would greatly benefit from parameterized types! Just look at most of the
functions, they look like they could work equally well on any pointer types or
with minimum effort!
Thus, the library becomes "A text processing library, plus a
few random functions." I would just remove those, rename fslurp to
slurp_read, and rename everything else slurp_* (minus the 'f').

But for the time being, I wouldn't be so concerned about purging fslurpl(), I
have long wanted to implement it :) If only C had parameterized types (sigh).
The
error indications are a bit lacking. For example, fsave does not
distinguish between a file error (fopen) or a stream error (fputs/c). It
also would be nice, from a maintenance perspective, to list the
functions in f.c in alphabetical order with the function name at the
start of the line:

I absolutely agree. But I really don't want to get fsave() to return a troolean
value to distinguish among open errors, write errors and success. In any case,
the caller can be sure that if fsave() returns nonzero, then not all the data
made it to its destination.
 
D

David Rubin

rihad said:
Well, it's quite obvious that the memory is returned dynamically allocated.

Only to you :) There is no accounting for what other people find to be
obvious.

/david
 
R

rihad

Only to you :) There is no accounting for what other people find to be
obvious.

Ok, you win :) Added a note to use ffree(). Thanks a lot for taking the time!

I hope other than that everything is fine.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,764
Messages
2,569,567
Members
45,041
Latest member
RomeoFarnh

Latest Threads

Top