code critique request

Discussion in 'C Programming' started by rihad, Oct 21, 2003.

  1. rihad

    rihad Guest

    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.
     
    rihad, Oct 21, 2003
    #1
    1. Advertising

  2. rihad

    David Rubin Guest

    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
     
    David Rubin, Oct 21, 2003
    #2
    1. Advertising

  3. rihad

    rihad Guest

    On Tue, 21 Oct 2003 11:17:42 -0400, David Rubin <>
    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.
     
    rihad, Oct 21, 2003
    #3
  4. rihad

    David Rubin Guest

    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
     
    David Rubin, Oct 21, 2003
    #4
  5. rihad

    rihad Guest

    On Tue, 21 Oct 2003 14:16:50 -0400, David Rubin <>
    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.
     
    rihad, Oct 22, 2003
    #5
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Fao, Sean

    Critique Request: CheckBoxColumn

    Fao, Sean, Feb 15, 2006, in forum: ASP .Net
    Replies:
    0
    Views:
    523
    Fao, Sean
    Feb 15, 2006
  2. Cynthia Turcotte

    critique request

    Cynthia Turcotte, Sep 12, 2003, in forum: HTML
    Replies:
    7
    Views:
    486
    Chris Leonard
    Sep 13, 2003
  3. Andrew Cameron

    Critique request: x01

    Andrew Cameron, Sep 14, 2003, in forum: HTML
    Replies:
    53
    Views:
    1,381
    picayunish
    Sep 17, 2003
  4. Jeff Dickens

    rubynuby code critique request

    Jeff Dickens, Dec 6, 2003, in forum: Ruby
    Replies:
    2
    Views:
    124
    Robert Klemme
    Dec 8, 2003
  5. James Edward Gray II

    Code Critique Request

    James Edward Gray II, Aug 26, 2004, in forum: Ruby
    Replies:
    7
    Views:
    160
    James Edward Gray II
    Aug 27, 2004
Loading...

Share This Page