Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > code critique request

Reply
Thread Tools

code critique request

 
 
rihad
Guest
Posts: n/a
 
      10-21-2003
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.

 
Reply With Quote
 
 
 
 
David Rubin
Guest
Posts: n/a
 
      10-21-2003
rihad wrote:

> 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

--
Andre, a simple peasant, had only one thing on his mind as he crept
along the East wall: 'Andre, creep... Andre, creep... Andre, creep.'
-- unknown
 
Reply With Quote
 
 
 
 
rihad
Guest
Posts: n/a
 
      10-21-2003
On Tue, 21 Oct 2003 11:17:42 -0400, David Rubin <(E-Mail Removed)>
wrote:

>rihad wrote:
>
>> 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*.
>


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.

>> 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.


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.

 
Reply With Quote
 
David Rubin
Guest
Posts: n/a
 
      10-21-2003
rihad wrote:

> >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.


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

/david

--
Andre, a simple peasant, had only one thing on his mind as he crept
along the East wall: 'Andre, creep... Andre, creep... Andre, creep.'
-- unknown
 
Reply With Quote
 
rihad
Guest
Posts: n/a
 
      10-22-2003
On Tue, 21 Oct 2003 14:16:50 -0400, David Rubin <(E-Mail Removed)>
wrote:

>rihad wrote:
>
>> >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.

>
>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.


 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Critique Request: CheckBoxColumn Fao, Sean ASP .Net 0 02-15-2006 05:09 PM
Code Critique Request James Edward Gray II Ruby 7 08-27-2004 03:28 PM
rubynuby code critique request Jeff Dickens Ruby 2 12-08-2003 12:34 PM
Critique request: x01 Andrew Cameron HTML 53 09-17-2003 08:56 PM
critique request Cynthia Turcotte HTML 7 09-13-2003 07:33 PM



Advertisments