Re: Derived Objects and OOP

[Robert T. Short]

Thanks for the response.  My responses are below.

Bob

John W. Eaton wrote:

On 14-Mar-2009, Robert T. Short wrote:

| Attached is a changeset and some associated tests and commentary for a 
| partial implementation of derived classes.
| 
| There are some "FIXME" comments in the code (2) for things I really 
| don't know how to do.
| 
| The attached tgz file include "NotesOnClasses" that describes what I 
| have done and why.  Also in the tgz file is a test script "ClassTest.m". 
| 
| There is still one major thing that has to be done before this is really 
| useful.  Assigning the result of a method in a parent class to the 
| derived class doesn't work.  See the test script "ClassTest.m" for 
| examples.  As yet I have not figured enough of this code out to have any 
| idea how to do this.  In the "NotesOnClasses" file are other issues that 
| have to be resolved, but mostly they are secondary in the sense that it 
| will be possible to write useful scripts without the change.
| 
| Comments are solicited:
| 
| 1.  I am not sure I am really using mercurial correctly.  I made my 
| changes, then
| 
|  > hg commit
|  > hg export changesetid > changeset
| 
| 2.  I tried to emulate the coding style, but probably missed.
| 
| 3.  Maybe this is all nonsense and there is a better way?
| 
| This should certainly not be included in 3.2 since it is incomplete and 
| since the number of files touched is small, I am not sure the patches 
| should even be applied yet, but I would really like comments.

I applied the patch with some additional changes, including adding
ChangeLog entries.  I'd like to have this in 3.2, primiarily because I
know of some code that I would like to see running in Octave that
relies on the inheritance features.  I know, I've been arguing for not
delaying the release by adding new features, but I don't think this is
too disruptive because classes are a new feature so a regression in
behavior from 3.0 isn't likely to be due to adding inheritance, and it
shouldn't cause trouble for classes that don't make use of the
inherintance feature.
  

Cool.  I am fairly desperate to get this into octave since I have to use MATLAB until it is there.  The only thing that seems to need regression testing is to ensure I didn't break the behavior of private functions.  I doubt it.

| Finally, if the person that wrote the original code has sample scripts 
| and classes that could be shared, then I can run regression tests and 
| eventually build a good integrated test suite.

Unfortunately, I don't have any test suite.
  

OK.  I will cobble something together from the documentation.

Quoting from the notes file included with the tarball:

| - Do private functions work right?

I haven't checked, and am not sure what needs to be checked.  Can you
write some tests for the features you expect to work?
  

What I meant by this comment is that I haven't checked to ensure that I didn't break anything.  I will do so as time permits and create proper test scripts.

| - load/save

What is supposed to be saved?
  

Saving an object should save the data for the derived class and all parents.

| - Assigning a class to the result of a parent method doesn't work.  See
|   the script "ClassTest.m" for examples.

I don't understand what you mean by this.
  

This is actually very important.  IMHO, without this feature the implementation is only half-baked.

Here is what I mean by this (look at the examples provided in the tarball for details on the class implementation).

Consider a class hierarchy

ClassName DirectoryName   Parent  Method
Snork     @Snork          None    gick (set and return the value of the property gick)
Dork      @Dork           Snork   gack (set and return the value of the property gack)

The following code fragment, taken from the ClassTest.m script, creates a Snork object with the default value of gick, and then changes the value of gick to 2.  This works fine.
snk      = Snork();
snk      = gick(snk,2)

However, the next code fragment does not work.  What it should do is set the value of the gick property in the drk.Snork field to 2.  That is, after the second line is executed typing gick(drk) should return 2.
drk      = Dork()
drk      = gick(drk,2)   % This doesn't work.

Note that get/set works just fine.  However if a class has multiple parents there is no way to use get/set, and the get/set technique is really clumsy in some situations.

| - "clear classes" should clear the parent list in load-path.

OK.  Should we really be storing the parent info in the load-path, and
searching it in find_method?  Or should that logic be in the symbol
table lookup code, with the list of parent classes stored somewhere
else (perhaps in the symbol table)?  I'm not sure what is best, so I
left it alone for now.
  

I really don't know the answer to this.  I put it there mostly because I haven't got the symbol table module figured out.  However, load_path seems to be a reasonable place since that is where the method file search takes place.

| - "isa", "which" and ??? don't work right.

I made isa work by writing a __parent_classes__ function that can take
an object and return a cell array of parent class names.  Does Matlab
provide a function that returns a list of parent classes for a given
object?

  

I will look and see what MATLAB provides.  I have thought of doing what you did regardless of MATLAB's behavior, so this is great.  I will look and see what you did with isa.  Just to be sure we are communicating, here is what should happen.  Referring to the example above, isa(drk,"Dork") and isa(drk,"Snork") should both return true.

How does the "which" function need to change?
  

Let me defer this until later since I want to play with MATLAB a bit to remind myself.  I will put together a transcript of a MATlAB session.  I mostly use which for debugging my classes, so this isn't a critical item in my view.

| - Regression testing on polynomials or other tests defined by other
|   developers.

It would be great to have additional tests.  Since objects require
class functions defined in files in directories with special names,
this may require some changes to the test suite.  I suppose we can
just add new class code to the test directory and run some tests
there instead of embedding the tests in the source code as we would
prefer to do for most test functions.

  

Agreed.  This makes the test suite a little complicated.  Philosophically, no implementation is complete without a test suite and documentation.  I rarely have time to work on this except for a couple of hours on weekend mornings so it is going dreadfully slowly.  As I gain familiarity with sources it should be easier for me to contribute.

| - The "dispatch_class" member of the function class should probably
|   indicate the parent class, and another member should indicate 
|   the calling class (or something similar).

The dispatch_class name that is stored in the octave_user_function
object is the name of the class in which the function is defined.  Why
should that be changed?  How would it be useful to store parent class
information there?

  

Actually, dispatch_class is the name of the class that invoked the method.  Thus, using the example above, invoking gick(drk) results in dispatch_class being "Dork" and gick(snk) results in dispatch_class being "Snork".  So I was thinking of creating another element that is "Snork" regardless of the invoking class.

jwe