From octave-maintainers-request at bevo dot che dot wisc dot edu Thu Nov 20 09:26:11 2003
Subject: restructuring load-save
From: "John W. Eaton" <jwe at bevo dot che dot wisc dot edu>
To: octave-maintainers mailing list <octave-maintainers at bevo dot che dot wisc dot edu>
Cc: David Bateman <David dot Bateman at motorola dot com>
Date: Thu, 20 Nov 2003 09:25:46 -0600

On 20-Nov-2003, David Bateman <David dot Bateman at motorola dot com> wrote:

| I've been thinking on how to fix up the load/save issue as well, and I
| think this could also be fixed fairly easily by introducing into ov-base.h
| 
| void * oct_ascii_load (void);
| void * oct_ascii_save (void);
| void * oct_binary_load (void);
| void * oct_binary_save (void);
| 
| bool have_oct_ascii_load { return false; }
| bool have_oct_ascii_save { return false; }
| bool have_oct_binary_load { return false; }
| bool have_oct_binary_save { return false; }
| 
| and appropriate virtual functions in ov.h. Then modify ov-oct-{ascii,binary}.cc
| to do something like
| 
| bool
| save_ascii_data ......
| 
| {
|   if (val_arg->have_oct_ascii_save()) {
|      val_arg->oct_ascii_save();
|   } else
|     // The old version of the code
|   }
| }
| 
| bool
| load_ascii_data ......
| 
| {
|   if (val_arg->have_oct_ascii_load()) {
|      val_arg->oct_ascii_load();
|   } else
|     // The old version of the code
|   }
| }

What is the void* return type for the save/load functions for?  You
don't seem to be using it here.

| The the user type can overload the oct_* methods and setup the
| have_oct_* functions appropriately. Similar things could also be done
| for the Matlab and HDF file formats.

Yes, this is a start, but let's examine it a bit more. First, let's
look at the save functions, because they are probably easier (we
already have an object, so we know its type and can call a method for
it).

If you are going to use virtual functions, then I think the default
save functionality should be in the octave_base_value class in
ov-base.{h,cc}.  Then you wouldn't need the have_*_save functions,
because if a derived class did not provide it's own save or load
function, you would get the default version in the octave_base_value
class.  Since we would be splitting all the code in the current
load-save functions up, I don't think there would be any old version
of the code left, so we would probably just be left with a "unable to
save objects of type XXX" error in the base code.  This is essentially
trading a switch statement in the current code for virtual function
dispatch, which is generally good, because extending it for new
objects is potentially cleaner.

For the load functions, things are more difficult, because you don't
know the type of the object until you read something from the file.
But perhaps this is not too much of a problem because I think each
file format includes some header information for each object that
tells us what the type is, so we could do something like

  octave_name_and_value_struct
  load_oct_binary_object (ostream& os, ...)
  {
    // get the name from the file along with a dummy object of the
    // appropriate type (used for dispatching).
    octave_name_and_value_struct tmp = read_object_info (os);
    tmp.val.oct_load (os);
    return tmp;
  }

This function would be called repeatedly until an error occurs or we
are out of values (signaled by returning an empty name and value
struct?).  The information returned could be added to an octave_value
struct array or inserted in the symbol table as needed.

Now, the only remaining problem is how to make load work for new
types?  Obviously, the read_object_info function can't know everything
about every type that might be added to Octave later, so it needs to
get some information from the file and pass it to each of the possible
octave_value subtypes in some reasonable order and allow them to
decide whether they are the appropriate object type to handle the
request.  Or, I suppose this could also be done with a lookup table
that is filled in when the various types are installed in the
interpreter.  We would also need some way of mapping MAT file type
information to specific octave_value types.

| Would such a change be acceptable? If so I wouldn't might coding it up...

Maybe.

It happens that I am currently in the process of splitting up the
load-save code into separate files based on file type, and trying to
make sure that we support all (or most) data types for each file
type.  Splitting it up further would also be OK, but I'd like to agree
that we have a good plan first.

jwe