Re: [boost] Filesystem Library Review

From: Beman Dawes (bdawes_at_[hidden])
Date: 2002-09-20 10:50:00

At 07:33 AM 9/20/2002, John Maddock wrote:

 >The following comments are mostly based on a review of the filesys lib
 >docs.
 >
 >Let me say up front that I like the library and think that it should be
 >accepted into boost.

Thanks!

 >Path.hpp
 >~~~~~~
 >
 >I don't really like the operator << usage, personally I would have
 >preferred:
 >
 >path::append(const path&);
 >path::operator+=(const path&);
 >path operator+(const path&, const path&);

Those were the first operators considered, and were used in the early 
experiments. The problem was that you often write sub-expressions like:

    "foo" + "bar"

That is confusing because "+" is associated in my mind with string 
concatenation, and yet that isn't what is going on here.

I think it is a mistake to hijack an operation with somewhat different 
semantics from the usual meaning in the context.

With operator<< (or Dave's suggestion of operator/), the sub-expression 
becomes:

    "foo" << "bar"
    "foo" / "bar"

Because these operations don't have traditional meanings for strings in 
most languages, it is clear that something special is going on. That's what 
drove the change to operator<<= and <<.

 >It's a pity that there are two different functions: file_path and
 >directory_path, these look error prone in usage to me: on most platforms
 >they will always return the same thing so potential bugs will never be
 >caught.  I can't really see any fix though :-(

Agreed. The use of those two functions is to interface with operating 
system or third-party library API's. Use in other contexts is probably a 
programming error.

I've added a warning to the docs.

 >All the functions generic_path, file_path, directory_path, leaf, and
 >branch,
 >are under-documented IMO. I had to read the specs quite closely before I
 >could figure out which did what.  Adding examples to each would probably 
be
 >a help, or maybe a "description" section that provides a less terse
 >description than the standardese.

I've added the above to the "do list"

 >I agree with comments made elsewhere that we need an "is_absolute"
 >function, in spite of the difficulties.

I'm committed to more query, tease apart, and combine functions, per the 
thread started by Thomas Witt.

 >operation.hpp
 >~~~~~~~~~
 >
 >Question: how does remove interact with directories?  Will it remove
 >non-empty directories, it should say so one way or the other.  OK reading 

 >it again I see:
 >
 >Throws: exists(ph) && is_directory(ph) && !is_empty(ph)
 >
 >which I assume means it should throw for non-empty directories?

Yes.

 >  Shouldn't
 >the throw clause say what it throws, followed by when.  Otherwise it 
reads
 >like the function always throws a bool ! :-)

Docs fixed by adding the word "if", as in all other "Throws" clauses. The 
front-matter already says that filesystem_error is the exception thrown.

 >Question: what's the difference between remove and remove_all?
 >
 >Comment: I think that the semantics of initial_directory are almost
 >useless,
 >I what to know what the current working directory is *right now*, not 
what
 >it was at some unspecified time in the past.  I would also like to be 
able
 >to set the present working directory when required (I realise that there
 >are OS's where this may not be possible, but no-one said the function
 >always had to succeed).

If someone is used to working with global variables like "current 
directory", as most programmers who do a lot of scripting are, it is hard 
for them to get their mind around the ideas that global variables are 
harmful, that it is even possible to write programs without them, and that 
the programs are often actually easier to write, read, debug, and maintain 
without them.

But it does take some getting used to.

Last winter, people have posted several examples that absolutely required 
get/set current directory. They all involved interfacing with operating 
system or third-party libraries which implicitly require that 
functionality. I may get talked into adding the get/set functionality, but 
only under extreme pressure and only in a way that discourages casual use.

 >Missing functions
 >~~~~~~~~~~~
 >
 >These are offered by way of a wish list, I don't see any need for these
 >functions to be added right now:
 >
 >Access to the CMA times of a file.
 >Access to the size of a file.
 >Access to the attributes of a file (probably only read/write/execute
 >permissions).
 >Ability to set the attributes of a file (probably only read/write/execute
 >permissions, and only for the current user).

Dietmar Kuehl and Jan Langer have done work on attributes/properties. 
Originally they were going to be part of the Filesystem Library proposal. 
But that became unworkable because the attributes design depends on the 
filesystem design, and it just became easier to do the current filesystem 
part first.

 >
 >Missing examples
 >~~~~~~~~~~~
 >
 >These are by way of a wish list, I think that their addition would help
 >verify that the library does in fact cover all the necessary bases, but I
 >don't see any need to insist on them right now:
 >
 >Implementations of the unix utilities: mv, cp, ls.

Yes, they might make good examples.

 >Nice work Beman!

Thanks, and thanks for all your comments!

--Beman