Re: [AD] Splitting filesystem APIs

[Thomas Fjellstrom <tfjellstrom@xxxxxxxxxx>]

On Sun April 19 2009, Peter Wang wrote:
> 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.

Except once you split the two types, you can't return any kind of handle from 
readdir, remember it can return files and directories. What exactly is it 
returning in your code examples?

Also your code is rather less realistic than you'd like to think. If you 
already have a handle, whats the point in using a second one via 
al_load_bitmap? Now add some actual logic to your code to handle all the 
errors, and that loading code is probably going to want to look at the stat 
info.

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

Really? Because you seem to do it a lot.

> Peter
>


-- 
Thomas Fjellstrom
tfjellstrom@xxxxxxxxxx