Re: [Ocaml-lib-devel] Future of ExtLib

[Bardur A. <oca...@sc...>]

On Fri, Dec 17, 2004 at 09:13:04PM +1100, skaller wrote:

> On Fri, 2004-12-17 at 20:09, Bardur Arantsson wrote:
> > > http://webperso.easyconnect.fr/gildor/ocaml-fileutils.html
> > 
> > This looks *very* interesting and the license seems to be
> > ExtLib-compatible. :)
> > 
> > However, for glancing at the code there are a couple of
> > minor issues:
> > 
> > - It uses lexers and parsers for reducing path names to an
> >   internal representation which seems like overkill. (Or
> >   is it just me?)
> 
> Doing things 'the right way' can't be overkill can it?
> I would expect this to be lightning fast and it should
> make it easy to generalise/extend ..?

I'm not sure there will ever be any need to
generalise/extend, but anyway...

To my mind, using full-blown parsers is overkill for
splitting UNIX paths into their constituent parts, but
I guess that's just an implementation detail, so let's
ignore that for the moment.

Another concern, which is more related to the interface is
that the module seems to raise exceptions in situations
one wouldn't normally expect. An an example:

    FilePath.check_extension p ext

raises an exception if the filename doesn't have an
extension. I can't tell whether this is a part of the
interface of check_extension since no ocamldoc comment is
present, but it certainly is *not* following the principle
of least surprise. That's the kind of thing that can be
*really* annoying to a library user.

Apart from that, I feel that simple 'shortcut' path
queries like is_dir, is_link, etc. should be added to the
FilePath module. I realize that this removes the
separation of purely abtract paths and concrete files, but
it's just too convenient to pass up IMO. Constrast

     if Path.is_dir p then

with

     if FileUtil.test FileUtil.Is_dir p then

Not a huge difference, but it is significant enough, IMO.
     
> 
> > - The FileUtil module seems a bit rough around the edges,
> 
> Perhaps -- but I think the interface is the primary
> concern .. the implementation can always be improved later.

You're right, of course.

All of the above is stuff that's fixable, so I guess the
best idea would be to just decide on an answer to the
question

     Do we want a path/file query/manipulation module in ExtLib?     

My answer would definitely be 'yes' (regardless of
which particular module it would end up being). This is an
area where the standard library is very weak and such a
module would be useful to most people using ExtLib.
     
I would be quite happy to use FilePath/FileUtil as the
basis for such a module.

> 
> > ... which brings up another thing which ExtLib is missing:
> > A test suite (and some mechanism for executing it).
> 
> I raise that issue before -- as I recall, Nicolas correctly
> said that's it is a *big* job.

Agreed, but IMHO it is becoming increasingly necessary as
the amount of code in ExtLib grows. Just an example from
real life: When I was writing OptParse I would have a test
program invoke the parser with various test cases just to
make sure that the various modifications I was doing
didn't create new bugs. When it was included in ExtLib,
those test cases couldn't be included and any future
modifications would run the risk of causing regressions.
This may also cause people who don't know the code
intimately to be relucant to try to improve/fix it. (In
some cases this even extends to the original authors if
they haven't looked at the code in question for a long
time).

> For Felix, I merge the tests and tutorial.. an added incentive.
> Learn how to use Extlib by studying the test cases.. 

I'm not sure how necessary an ExtLib tutorial is, but that
sound like a good idea. :)

> But it is a big job .. more is involved that just writing
> the test case (a test harness is needed too ..)

Maybe I'm just stupid, but I don't see why a test harness
would require lots of work...?

My suggestion would be something like this: Use OUnit for
running individual test cases/sets, add a lazy global
'test_cases' value to all the relevant modules. Finally,
add a Test module containing something along the lines of

       let all_test_cases = 
             ... [ Lazy.force ExtString.test_cases;
                   Lazy.force OptParse.test_cases;
                   ... ]
       in let counters =
          OUnit.run_test_tt_main all_test_cases
       in let rc =
          if counters.errors > 0 then 1 
          else if counters.failures > 0 then 2 
          else 0
       in
         printf "statistics: ....";
         exit rc;

This would enable all the installation methods to run the
test cases with very little effort.

Of course, writing individual test cases for all the
modules would be a lot of work, but this is work that can
be done incrementally.

Am I missing something?

-- 
Bardur Arantsson
<ba...@im...>
<ba...@sc...>