Re: [AD] Splitting filesystem APIs

[Peter Wang <novalazy@xxxxxxxxxx>]

On 2009-04-19, Thomas Fjellstrom <tfjellstrom@xxxxxxxxxx> wrote:
> On Sat April 18 2009, Elias Pschernig wrote:
> > He also already said that it won't be a big change from how things
> > currently work - the biggest change likely is the ALLEGRO_FILE type - which
> > should be an obvious change if you look at the list above. (I wouldn't
> > totally rule out that we keep ALLEGRO_FS_HANDLE after all, especially if
> > someone can come up with an example where it is much easier to work it -
> > but right now I'm also seeing more arguments against than for it.)
> 
> Its been so long since I did all the original research, planning and work that 
> I myself don't really remember all the reasons. I really do wish people had 
> spoken up a heck of a lot sooner. I'd be a lot LOT less frustrated right now.

I can think of three patterns of usage when dealing with files.

1. You know the name of the file you will work with.
You open the file, read/write, close the file.

2. You scan a directory for a list of files.

3. You scan a directory for a list of files, then based on some criteria
open the current directory entry, work with it, close it.

(1) and (2) will be discussed later.

Case (3) is the one that seems like it might benefit from a combined
ALLEGRO_FS_ENTRY.  But when scanning a directory, once you decide to work
with a file, you still need to open it, read/write, then close it.

Before, you might have code like this:

    next = al_readdir(entry);
    if (!next)
        break;
    if (al_open_entry(next, "rb")) {
        al_load_bitmap_entry(next, "bmp");
        al_close_entry(next);
    }
    al_destroy_entry(next);

Now, it's only slightly different:

    next = al_readdir(entry);
    if (!next)
        break;
    filename = al_path_to_string(al_get_entry_name(next), '/');
    f = al_fopen(filename, "rb");
    if (f) {
        al_load_bitmap_entry(f, "bmp");
        al_fclose(f);
    }
    al_destroy_entry(next);

or more realistically,

    next = al_readdir(entry);
    if (!next)
        break;
    filename = al_path_to_string(al_get_entry_name(next), '/');
    al_load_bitmap(filename, "bmp");
    al_destroy_entry(next);

The calls to al_open_entry/al_fopen and al_close_entry/al_fclose correspond
exactly!  There is some ugliness because right now al_get_entry_name() happens
to return an ALLEGRO_PATH instead of a string, but that's easily solved.
Basically, no real impact either way.


What about case (1)?  Now it can work with ALLEGRO_FILEs exclusively.  The
writer of the code knows that an ALLEGRO_FILE is definitely open, and when
searching for functions only has to consider the smaller subset of functions
that take an ALLEGRO_FILE argument: easier to understand, just as convenient.

Case (2)?  Now it works with ALLEGRO_FS_ENTRYs exclusively.  Again, the writer
only needs to consider a smaller subset of functions that work with
ALLEGRO_FS_ENTRYs.  He won't accidentally call any functions that only make
sense for an open file, and the compiler will make sure of it.  Easier to
understand, just as convenient.

Your work hasn't been discarded.  It handles case (2) unchanged,
and case (1) and (3) with only a change in the type.  Maybe I managed to
convince you of it, and you can feel a bit less frustrated.  Pulling
rank happens to feel shit as well.

Peter