Objects and OOP

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

Objects and OOP

Robert T. Short
I am new to this list, so forgive me if I am rehashing old topics.

I downloaded the 3.1.51+ sources and have been experimenting with the
OOP facilities.  I have two questions.

It seems that support for basic objects is present, but support for
inheritance is not.  True?

If true, then my second question is: can I help implement the
inheritance features?

Bob
--
Robert T. Short
PhaseLocked Systems

Reply | Threaded
Open this post in threaded view
|

Re: Objects and OOP

David Bateman-2
Robert T. Short wrote:
> I am new to this list, so forgive me if I am rehashing old topics.
>
> I downloaded the 3.1.51+ sources and have been experimenting with the
> OOP facilities.  I have two questions.
>
> It seems that support for basic objects is present, but support for
> inheritance is not.  True?
True,
>
> If true, then my second question is: can I help implement the
> inheritance features?

Sure, I believe the OOP features already do what John wants so I don't
think he'll do the rest. However, you might prefer to look at the
classdef support rather than inheritance in the older OOP code. I
believe Michael Goffioul send some code with the start of this in November..

D.


--
David Bateman                                [hidden email]
35 rue Gambetta                              +33 1 46 04 02 18 (Home)
92100 Boulogne-Billancourt FRANCE            +33 6 72 01 06 33 (Mob)

Reply | Threaded
Open this post in threaded view
|

Re: Objects and OOP

Michael Goffioul
On Thu, Jan 8, 2009 at 9:02 PM, David Bateman <[hidden email]> wrote:
> Sure, I believe the OOP features already do what John wants so I don't think
> he'll do the rest. However, you might prefer to look at the classdef support
> rather than inheritance in the older OOP code. I believe Michael Goffioul
> send some code with the start of this in November..

Nothing functional, I'm afraid. It was basically a skeleton implementing
meta-object protocol. I didn't have time to continue further yet.

Michael.
Reply | Threaded
Open this post in threaded view
|

Re: Objects and OOP

Ryan Rusaw
On Thu, Jan 8, 2009 at 3:38 PM, Michael Goffioul
<[hidden email]> wrote:

> On Thu, Jan 8, 2009 at 9:02 PM, David Bateman <[hidden email]> wrote:
>> Sure, I believe the OOP features already do what John wants so I don't think
>> he'll do the rest. However, you might prefer to look at the classdef support
>> rather than inheritance in the older OOP code. I believe Michael Goffioul
>> send some code with the start of this in November..
>
> Nothing functional, I'm afraid. It was basically a skeleton implementing
> meta-object protocol. I didn't have time to continue further yet.
>
> Michael.
>

I had some idle time over my 2 weeks off for the winter holiday
season, so I added classdef support to the parser and lexer as well as
preliminary +package directory support.

I've only just started the work on fleshing out Michael's meta-object
code and integrating it with the parser though, so it's far from being
complete as well.

Ryan
Reply | Threaded
Open this post in threaded view
|

Re: Objects and OOP

John W. Eaton
Administrator
On  8-Jan-2009, Ryan Rusaw wrote:

| On Thu, Jan 8, 2009 at 3:38 PM, Michael Goffioul
| <[hidden email]> wrote:
| > On Thu, Jan 8, 2009 at 9:02 PM, David Bateman <[hidden email]> wrote:
| >> Sure, I believe the OOP features already do what John wants so I don't think
| >> he'll do the rest. However, you might prefer to look at the classdef support
| >> rather than inheritance in the older OOP code. I believe Michael Goffioul
| >> send some code with the start of this in November..
| >
| > Nothing functional, I'm afraid. It was basically a skeleton implementing
| > meta-object protocol. I didn't have time to continue further yet.
| >
| > Michael.
| >
|
| I had some idle time over my 2 weeks off for the winter holiday
| season, so I added classdef support to the parser and lexer as well as
| preliminary +package directory support.
|
| I've only just started the work on fleshing out Michael's meta-object
| code and integrating it with the parser though, so it's far from being
| complete as well.

Please consider posting patches as you complete small parts of this
work.  Integrating those would be easier for us than if you wait and
send some large patch that touches many files.

jwe
Reply | Threaded
Open this post in threaded view
|

Re: Objects and OOP

Robert T. Short
John W. Eaton wrote:

> On  8-Jan-2009, Ryan Rusaw wrote:
>
> | On Thu, Jan 8, 2009 at 3:38 PM, Michael Goffioul
> | <[hidden email]> wrote:
> | > On Thu, Jan 8, 2009 at 9:02 PM, David Bateman <[hidden email]> wrote:
> | >> Sure, I believe the OOP features already do what John wants so I don't think
> | >> he'll do the rest. However, you might prefer to look at the classdef support
> | >> rather than inheritance in the older OOP code. I believe Michael Goffioul
> | >> send some code with the start of this in November..
> | >
> | > Nothing functional, I'm afraid. It was basically a skeleton implementing
> | > meta-object protocol. I didn't have time to continue further yet.
> | >
> | > Michael.
> | >
> |
> | I had some idle time over my 2 weeks off for the winter holiday
> | season, so I added classdef support to the parser and lexer as well as
> | preliminary +package directory support.
> |
> | I've only just started the work on fleshing out Michael's meta-object
> | code and integrating it with the parser though, so it's far from being
> | complete as well.
>
> Please consider posting patches as you complete small parts of this
> work.  Integrating those would be easier for us than if you wait and
> send some large patch that touches many files.
>
> jwe
>
>
>  
Sounds good!

Bob
--
Robert T. Short
PhaseLocked Systems
Reply | Threaded
Open this post in threaded view
|

Objects and OOP

John W. Eaton
Administrator
In reply to this post by Robert T. Short
On  8-Jan-2009, Robert T. Short wrote:

| I am new to this list, so forgive me if I am rehashing old topics.
|
| I downloaded the 3.1.51+ sources and have been experimenting with the
| OOP facilities.  I have two questions.
|
| It seems that support for basic objects is present, but support for
| inheritance is not.  True?

Can you post an example of what you expect to work?

| If true, then my second question is: can I help implement the
| inheritance features?

Yes.

jwe
Reply | Threaded
Open this post in threaded view
|

Re: Objects and OOP

Robert T. Short
Sorry to be so slow.  There is a lot of code to understand before I try
to make any changes.  I am probably a little overcautious.  Also, like
everybody, somewhat time-constrained.

I am building a test case by stripping down a very complex MATLAB system
that I have been building for a client.  The result will be a somewhat
contrived but fairly complete test suite.  Attached is a very
preliminary version - just single inheritance.  The next step will be to
add the appropriate support in "isa()".  Multiple and multi-level
inheritance will come as I am able to get it done.  There are some
things that I haven't figured out how to do yet but since basic class
support is present I don't really think this is a hard problem - or at
least wouldn't be if I was more familiar with the sources.  I think
aggregation will be easier than inheritance.

The way I am approaching this is to implement the "old-style" OOP
interface first; class methods are all in a directory with name
@ClassName.  There are two reasons for this.  First, my MATLAB is a
little long in the tooth and doesn't have classdef-style OOP support,
and second I don't have to muck about with the parser much (not at
all?).  I think classdef support will be able to build on the old-style
stuff without much code support - just parser support.  In the end, I
would like to have a very complete MATLAB style OOP system.

I have done some modification to ov-class.{h,cc}, and hope to have
something functional to post by the weekend.  I certainly won't have the
complete solution that soon, but you did ask me to do small chunks.  I
really want to get this to work so I can quit using Windows and back to
my linux system.


Bob
--
Robert T. Short
PhaseLocked Systems

John W. Eaton wrote:

> On  8-Jan-2009, Robert T. Short wrote:
>
> | I am new to this list, so forgive me if I am rehashing old topics.
> |
> | I downloaded the 3.1.51+ sources and have been experimenting with the
> | OOP facilities.  I have two questions.
> |
> | It seems that support for basic objects is present, but support for
> | inheritance is not.  True?
>
> Can you post an example of what you expect to work?
>
> | If true, then my second question is: can I help implement the
> | inheritance features?
>
> Yes.
>
> jwe
>
>
>  


classtest.tgz (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Objects and OOP

John W. Eaton
Administrator
On 27-Jan-2009, Robert T. Short wrote:

| I think classdef support will be able to build on the old-style
| stuff without much code support - just parser support.  In the end, I
| would like to have a very complete MATLAB style OOP system.

I thought it was completely different, because it has reference
semantics, and the current class thing is value based.

| I have done some modification to ov-class.{h,cc}, and hope to have
| something functional to post by the weekend.  I certainly won't have the
| complete solution that soon, but you did ask me to do small chunks.  I
| really want to get this to work so I can quit using Windows and back to
| my linux system.

I could probably help with the implementation if I understood clearly
how inheritance is supposed to work.

jwe
Reply | Threaded
Open this post in threaded view
|

Re: Objects and OOP

Robert T. Short
John W. Eaton wrote:
> On 27-Jan-2009, Robert T. Short wrote:
>
> | I think classdef support will be able to build on the old-style
> | stuff without much code support - just parser support.  In the end, I
> | would like to have a very complete MATLAB style OOP system.
>
> I thought it was completely different, because it has reference
> semantics, and the current class thing is value based.
>  
Well, I could be wrong.  In fact, it is likely.  What I did was look at
the existing code, the MATLAB documentation, the octave sources, and
what I really need right now.  I said "Bob, what you can do easily
is.....".  If I was wrong about the classdef stuff, then I will probably
have to do the same work twice.  On the other hand the first time
working with somebody else's code, it is likely better to go simple and
let the chips fall where they may.  Furthermore, if the goal is to be
MATLAB compatible (and it is a serious goal for me), then both are
necessary.
> | I have done some modification to ov-class.{h,cc}, and hope to have
> | something functional to post by the weekend.  I certainly won't have the
> | complete solution that soon, but you did ask me to do small chunks.  I
> | really want to get this to work so I can quit using Windows and back to
> | my linux system.
>
> I could probably help with the implementation if I understood clearly
> how inheritance is supposed to work.
>  

I expect you could do the whole thing in 15 minutes.  If you prefer, I
can just dump what I am trying to do and you can do it or just tell me
how.  It is probably worth my time and yours for me to poke around and
learn the sources and then have you critique my solutions.  Slow and a
bit painful, but that is the only way I know of to learn the code and it
gets it off of your shoulders.  I am just now getting to the point where
I am modifying sources, and will probably have questions and I hope you
don't mind.  Of course I will document it when I am done.

Octave is quite complex.  I hope you appreciate the astonishing code
base you have built.  I sure do.


> jwe
>
>
>  
Bob
--
Robert T. Short
PhaseLocked Systems

Reply | Threaded
Open this post in threaded view
|

Derived Objects and OOP

Robert T. Short
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.

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.


Bob
--
Robert T. Short
PhaseLocked Systems

# HG changeset patch
# User [hidden email]
# Date 1237051272 25200
# Node ID 2e94632e5f7a2e02b59f529adeb9a85de05de458
# Parent  2e9af33636694e611a08e4f00c70c13491d180b7
Partial implementation of derived classes using the old form with "@" files.

diff -r 2e9af3363669 -r 2e94632e5f7a src/load-path.cc
--- a/src/load-path.cc Fri Mar 13 16:47:50 2009 +0100
+++ b/src/load-path.cc Sat Mar 14 10:21:12 2009 -0700
@@ -512,6 +512,7 @@
   fcn_map.clear ();
   private_fcn_map.clear ();
   method_map.clear ();
+  parent_map.clear ();
 
   do_append (".", false);
 }
@@ -1016,7 +1017,23 @@
 
       const_fcn_map_iterator p = m.find (meth);
 
-      if (p != m.end ())
+      if (p == m.end ())   // Look in parent classes.
+ {
+  const_parent_map_iterator r = parent_map.find (class_name);
+
+  if ( r != parent_map.end () )
+    {
+      std::list<std::string> plist = r->second;
+      std::list<std::string>::iterator it = plist.begin();
+      while ( it != plist.end() )
+ {
+  retval = do_find_method(*it, meth, dir_name, type);
+  if (retval != "") break;
+  it++;
+ }
+    }
+ }
+      else
  {
   const file_info_list_type& file_info_list = p->second;
 
@@ -1764,6 +1781,12 @@
   execute_pkg_add_or_del (dir, "PKG_DEL");
 }
 
+void load_path::do_add_to_parent_map (std::string classname,
+      std::list<std::string> parent_list)
+{
+  parent_map[classname] = parent_list;
+}
+
 DEFUN (genpath, args, ,
   "-*- texinfo -*-\n\
 @deftypefn {Built-in Function} {} genpath (@var{dir})\n\
diff -r 2e9af3363669 -r 2e94632e5f7a src/load-path.h
--- a/src/load-path.h Fri Mar 13 16:47:50 2009 +0100
+++ b/src/load-path.h Sat Mar 14 10:21:12 2009 -0700
@@ -37,7 +37,7 @@
 {
 protected:
 
-  load_path (void) : dir_info_list (), fcn_map (), method_map () { }
+  load_path (void) : dir_info_list (), fcn_map (), method_map (), parent_map () { }
 
 public:
 
@@ -228,6 +228,12 @@
     return instance_ok () ? instance->do_system_path () : std::string ();
   }
 
+  static void add_to_parent_map (std::string classname, std::list<std::string> parent_list)
+  {
+    if ( instance_ok () )
+ instance->do_add_to_parent_map (classname, parent_list);
+  }
+
 private:
 
   static const int M_FILE = 1;
@@ -386,6 +392,12 @@
 
   typedef method_map_type::const_iterator const_method_map_iterator;
   typedef method_map_type::iterator method_map_iterator;
+
+  // <CLASS_NAME, PARENT_LIST>>
+  typedef std::map<std::string, std::list<std::string> > parent_map_type;
+
+  typedef parent_map_type::const_iterator const_parent_map_iterator;
+  typedef parent_map_type::iterator parent_map_iterator;
 
   mutable dir_info_list_type dir_info_list;
 
@@ -394,6 +406,8 @@
   mutable private_fcn_map_type private_fcn_map;
 
   mutable method_map_type method_map;
+
+  mutable parent_map_type parent_map;
 
   static load_path *instance;
 
@@ -493,6 +507,8 @@
 
   std::string do_get_command_line_path (void) const { return command_line_path; }
 
+  void do_add_to_parent_map (std::string classname, std::list<std::string> parent_list);
+
   void add_to_fcn_map (const dir_info& di, bool at_end) const;
 
   void add_to_private_fcn_map (const dir_info& di) const;
diff -r 2e9af3363669 -r 2e94632e5f7a src/ov-base.h
--- a/src/ov-base.h Fri Mar 13 16:47:50 2009 +0100
+++ b/src/ov-base.h Sat Mar 14 10:21:12 2009 -0700
@@ -128,6 +128,12 @@
   octave_base_value (const octave_base_value&) { }
 
   virtual ~octave_base_value (void) { }
+
+  virtual std::list<std::string> get_parent_list (void) const
+             { return std::list<std::string>(); }
+  // FIXME: should gripe_something even though this "can't happen"?
+  virtual octave_base_value* find_parent_class(const std::string)
+             { return 0; }
 
   virtual octave_base_value *
   clone (void) const { return new octave_base_value (*this); }
diff -r 2e9af3363669 -r 2e94632e5f7a src/ov-class.cc
--- a/src/ov-class.cc Fri Mar 13 16:47:50 2009 +0100
+++ b/src/ov-class.cc Sat Mar 14 10:21:12 2009 -0700
@@ -62,6 +62,68 @@
     (octave_class::t_name, "<unknown>", octave_value (new octave_class ()));
 }
 
+octave_class::octave_class (const Octave_map& m, const std::string& id,
+                octave_value_list& prnts)
+    : octave_base_value (), map (m), c_name (id)
+{
+  for(int idx=0; idx<prnts.length(); idx++)
+    {
+      if (! prnts(idx).is_object() )
+ error ("parents must be objects");
+      else
+ {
+  parent_list.push_back ( prnts(idx).class_name() );
+  map.assign (prnts(idx).class_name(), prnts(idx));
+ }
+    }
+  load_path::add_to_parent_map(id, parent_list);
+}
+
+octave_base_value*
+octave_class::find_parent_class(const std::string parent_class_name)
+{
+  octave_base_value* retval = 0;
+  std::string dbg_clsnm = class_name();
+  for(std::list<std::string>::iterator it=parent_list.begin();
+      it != parent_list.end();
+      it++)
+    {
+      std::string dbg_prnt = *it;
+    }
+
+  if ( parent_class_name == class_name() )
+    retval = this;
+  else
+    {
+      //  First look in the immediate parents.
+      std::list<std::string>::iterator
+ p = find(parent_list.begin(), parent_list.end(), parent_class_name);
+      if (p!=parent_list.end())
+ {
+  std::string dbg_prnt = *p;
+  Octave_map::const_iterator pmap = map.seek (parent_class_name);
+  if (pmap != map.end())  //  Don't really need this check.
+    {
+      retval = map.contents(pmap)(0,0).internal_rep();
+    }
+ }
+      else  // Now look in the ancestor tree.
+ {
+  for(std::list<std::string>::iterator pit=parent_list.begin();
+      pit != parent_list.end();
+      pit++)
+    {
+      std::string dbg_prnt = *pit;
+      Octave_map::const_iterator smap = map.seek (*pit);
+      octave_base_value* obvp = map.contents(smap)(0,0).internal_rep();
+      retval = obvp->find_parent_class(parent_class_name);
+      if (retval != 0) break;
+    }
+ }
+    }
+  return retval;
+}
+
 Cell
 octave_class::dotref (const octave_value_list& idx)
 {
@@ -69,12 +131,29 @@
 
   assert (idx.length () == 1);
 
+  // FIXME:  Is there a "proper" way to do this?
+  octave_function* fcn = octave_call_stack::current();
+  std::string my_dir = fcn->dir_name();
+  int ipos = my_dir.find_last_of("@");
+  std::string method_class = my_dir.substr(ipos+1);
+
+  // Find the class in which this method resides before attempting to access
+  // the requested field.
+
+  octave_base_value* obvp = find_parent_class(method_class);
+
+  Octave_map my_map;
+  if (obvp!=0)
+    my_map = obvp->map_value();
+  else
+    my_map = map;
+
   std::string nm = idx(0).string_value ();
 
-  Octave_map::const_iterator p = map.seek (nm);
+  Octave_map::const_iterator p = my_map.seek (nm);
 
-  if (p != map.end ())
-    retval = map.contents (p);
+  if (p != my_map.end ())
+    retval = my_map.contents (p);
   else
     error ("class has no member `%s'", nm.c_str ());
 
@@ -1167,19 +1246,24 @@
 DEFUN (class, args, ,
   "-*- texinfo -*-\n\
 @deftypefn {Built-in Function} {} class (@var{expr})\n\
+Return the class of the expression @var{expr}.\n\
 @deftypefnx {Built-in Function} {} class (@var{s}, @var{id})\n\
+Create a base class with fields from structure @var{s} and name (string) @var{id}.\n\
+@deftypefnx {Built-in Function} {} class (@var{s}, @var{id}, @var{p}, @dots{})\n\
+Create a class derived from parent classes @var{p} with fields from @var{s} and name @var{id}\n\
 \n\
-Return the class of the expression @var{expr}, as a string or\n\
-create a class object from the structure @var{s} with name @var{id}.\n\
 @end deftypefn")
 {
   octave_value retval;
 
   int nargin = args.length ();
 
+  if (nargin == 0)
+    print_usage ();
+
   if (nargin == 1)
     retval = args(0).class_name ();
-  else if (nargin == 2)
+  else
     {
       Octave_map m = args(0).map_value ();
 
@@ -1192,7 +1276,13 @@
       octave_function *fcn = octave_call_stack::caller ();
 
       if (fcn && fcn->is_class_constructor ())
- retval = octave_value (new octave_class (m, id));
+ if (nargin==2)
+  retval = octave_value (new octave_class (m, id));
+ else
+  {
+    octave_value_list prnts = args.slice(2,args.length()-2);
+    retval = octave_value (new octave_class (m, id, prnts));
+  }
       else
  error ("class: invalid call from outside class constructor");
     }
@@ -1202,8 +1292,6 @@
       else
  error ("class: expecting structure as first argument");
     }
-  else
-    print_usage ();
 
   return retval;
 }
diff -r 2e9af3363669 -r 2e94632e5f7a src/ov-class.h
--- a/src/ov-class.h Fri Mar 13 16:47:50 2009 +0100
+++ b/src/ov-class.h Sat Mar 14 10:21:12 2009 -0700
@@ -55,9 +55,14 @@
     : octave_base_value (), map (m), c_name (id) { }
 
   octave_class (const octave_class& s)
-    : octave_base_value (s), map (s.map), c_name (s.c_name) { }
+    : octave_base_value (s), map (s.map), c_name (s.c_name), parent_list (s.parent_list) { }
+
+  octave_class (const Octave_map& m, const std::string& id,
+                octave_value_list& prnts);
 
   ~octave_class (void) { }
+
+  virtual octave_base_value* find_parent_class (std::string);
 
   octave_base_value *clone (void) const { return new octave_class (*this); }
 
@@ -165,6 +170,7 @@
 
   static const std::string t_name;
   std::string c_name;
+  std::list<std::string> parent_list;
 
   bool in_class_method (void) const;
 };

octaveTest.tgz (12K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Derived Objects and OOP

John W. Eaton
Administrator
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.

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

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?

| - load/save

What is supposed to be saved?

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

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

| - "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?

How does the "which" function need to change?

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

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

jwe
Reply | Threaded
Open this post in threaded view
|

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
  
Reply | Threaded
Open this post in threaded view
|

Re: Derived Objects and OOP

John W. Eaton
Administrator
On 28-Mar-2009, Robert T. Short wrote:

| <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
| <html>

Can you disable the HTML goop when writing to the lists?

| Unfortunately, I don't have any test suite.
|   </pre>
| </blockquote>
| OK.  I will cobble something together from the documentation.
| <blockquote cite="mid:%[hidden email]%3E"
|  type="cite">
|   <pre wrap="">
| 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?
|   </pre>
| </blockquote>
| 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.

OK.

| | - load/save
|
| What is supposed to be saved?
|   </pre>
| </blockquote>
| Saving an object should save the data for the derived class and all
| parents.

Are you sure?  The description of the MAT-file format doesn't mention
parent class names.  It says that an object is stored the same as a
structure with the name of the class.  Octave already does this, so
I'm not sure there is anything to change.

| | - 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.
|   </pre>
| </blockquote>
| 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.
| drk      = Dork()
| drk      = gick(drk,2)   % This
| doesn't work.
|  What it <i>should</i>
| 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.

What's needed to make that happen?

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

Do you mean that it should be possible to use get/set in Octave, but
it is not working now, or that it is not possible in Matlab either?

| <blockquote cite="mid:%[hidden email]%3E"
|  type="cite">
|   <pre wrap="">
| | - "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.
|   </pre>
| </blockquote>
| 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.

OK.

| <blockquote cite="mid:%[hidden email]%3E"
|  type="cite">
|   <pre wrap="">
| | - "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?
|
|   </pre>
| </blockquote>
| 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.

Yes, that's the way it works now.

| | - 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.
|
|   </pre>
| </blockquote>
| 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.

Tests would be great to have, but right now it would help more if you
can explain how things are supposed to work (for example, how can we
implement the field setting thing above that you say is broken).

|   <pre wrap="">| - 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?
|
|   </pre>
| </blockquote>
| 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.
| <blockquote cite="mid:%[hidden email]%3E"
|  type="cite">
|   <pre wrap="">jwe
|   </pre>
| </blockquote>

So you are thinking of explicitly caching the base class name(s)?
That might help performance if the base class is always needed.  But
wouldn't it be sufficient to simply look up the base class name(s)
when they are needed?  Given a class name, isn't it possible to look
up the base class names(s) quickly?  Aren't they already stored in a
map?

jwe
Reply | Threaded
Open this post in threaded view
|

Re: Derived Objects and OOP

Robert T. Short
John W. Eaton wrote:
> On 28-Mar-2009, Robert T. Short wrote:
>
> | <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
> | <html>
>
> Can you disable the HTML goop when writing to the lists?
>  

Grumble.  This mail thing used to ask me about this.  I will watch this
in the future.  I am trying with this mail, but am not sure if I know
how to turn it off so let me know if it still comes in as HTML.

> | Unfortunately, I don't have any test suite.
> |   </pre>
> | </blockquote>
> | OK.  I will cobble something together from the documentation.
> | <blockquote cite="mid:%[hidden email]%3E"
> |  type="cite">
> |   <pre wrap="">
> | 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?
> |   </pre>
> | </blockquote>
> | 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.
>
> OK.
>
> | | - load/save
> |
> | What is supposed to be saved?
> |   </pre>
> | </blockquote>
> | Saving an object should save the data for the derived class and all
> | parents.
>
> Are you sure?  The description of the MAT-file format doesn't mention
> parent class names.  It says that an object is stored the same as a
> structure with the name of the class.  Octave already does this, so
> I'm not sure there is anything to change.
>  

No I am not sure.  I guess this was really a "note to Bob" to make sure
that things work sensibly.  I am not at all sure there IS anything to be
done, since saving the object should really just save the structure and
that is probably already done.  There might be something to do to make
sure that the load process re-creates the parent list.


> | | - 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.
> |   </pre>
> | </blockquote>
> | 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.
> | drk      = Dork()
> | drk      = gick(drk,2)   % This
> | doesn't work.
> |  What it <i>should</i>
> | 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.
>
> What's needed to make that happen?
>  

As far as implementation I have no clue (yet).  I have been looking
through all of the tree evaluation stuff just trying to make sure I know
what it does before I go tapdancing through the sources.

Using the example above, if the invocation is gick(drk), then the Snork
field of drk should be updated with the results of the gick function.  
In general, the parent tree must be traversed to find the appropriate
structure field for the class that owns the method and that structure
field updated with the result of the method function.

> | 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.
>
> Do you mean that it should be possible to use get/set in Octave, but
> it is not working now, or that it is not possible in Matlab either?
>  

Not possible in MATLAB.  Or more accurately, I haven't figured out a
clean way to do it.  For examples, see the classes I provided in the
tarball.  The way get/set works is that if the derived class doesn't
have a property name it invokes get/set on the parent structure field.  
If the parent doesn't have it, it will do it for all of its' parents.  
If you get to the top of the tree without finding the property name, an
error is thrown.  In the case of multiple inheritance, if you don't pick
the right parent to start with then an error will be thrown.  It would
be very simple to provide a clean extension in octave, and I am
considering this when the critical stuff works.

I don't really like the get/set approach for my own applications, so
this is not too high on my priority list.

> | <blockquote cite="mid:%[hidden email]%3E"
> |  type="cite">
> |   <pre wrap="">
> | | - "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.
> |   </pre>
> | </blockquote>
> | 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.
>
> OK.
>
> | <blockquote cite="mid:%[hidden email]%3E"
> |  type="cite">
> |   <pre wrap="">
> | | - "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?
> |
> |   </pre>
> | </blockquote>
> | 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.
>
> Yes, that's the way it works now.
>  


Cool.  I will include this in my test code.

> | | - 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.
> |
> |   </pre>
> | </blockquote>
> | 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.
>
> Tests would be great to have, but right now it would help more if you
> can explain how things are supposed to work (for example, how can we
> implement the field setting thing above that you say is broken).
>  

Exactly.  First make things work, then document/test.  I wish I could
spend more time, but my priorities are (1) billable hours, (2) billable
hours, (3) the book I am writing, (4) billable hours, (5) all of the
other projects.

> |   <pre wrap="">| - 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?
> |
> |   </pre>
> | </blockquote>
> | 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.
> | <blockquote cite="mid:%[hidden email]%3E"
> |  type="cite">
> |   <pre wrap="">jwe
> |   </pre>
> | </blockquote>
>
> So you are thinking of explicitly caching the base class name(s)?
> That might help performance if the base class is always needed.  But
> wouldn't it be sufficient to simply look up the base class name(s)
> when they are needed?  Given a class name, isn't it possible to look
> up the base class names(s) quickly?  Aren't they already stored in a
> map?
>  

Just thinking about it.  The key is to make things both simple and
quick.  Situations where there are dozens of parents are unlikely, so
this is a small issue.  However, every time I go looking through the
parent load path I have to dredge out the parent class file name, so it
might be useful.

> jwe
>
>
>  

Reply | Threaded
Open this post in threaded view
|

Re: Derived Objects and OOP

John W. Eaton
Administrator
On 28-Mar-2009, Robert T. Short wrote:

| John W. Eaton wrote:
| > On 28-Mar-2009, Robert T. Short wrote:
| >
| > | <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
| > | <html>
| >
| > Can you disable the HTML goop when writing to the lists?
| >  
|
| Grumble.  This mail thing used to ask me about this.  I will watch this
| in the future.  I am trying with this mail, but am not sure if I know
| how to turn it off so let me know if it still comes in as HTML.

I just got plain text this time.  Thanks.

| > Are you sure?  The description of the MAT-file format doesn't mention
| > parent class names.  It says that an object is stored the same as a
| > structure with the name of the class.  Octave already does this, so
| > I'm not sure there is anything to change.
|
| No I am not sure.  I guess this was really a "note to Bob" to make sure
| that things work sensibly.  I am not at all sure there IS anything to be
| done, since saving the object should really just save the structure and
| that is probably already done.  There might be something to do to make
| sure that the load process re-creates the parent list.

I'm not sure I see how that would be done.  What does Matlab do, if
the Matlab MAT file format doesn't seem to have any place to store
parent class info.

| > | | - 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.
| > |   </pre>
| > | </blockquote>
| > | 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.
| > | drk      = Dork()
| > | drk      = gick(drk,2)   % This
| > | doesn't work.
| > |  What it <i>should</i>
| > | 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.
| >
| > What's needed to make that happen?
| >  
|
| As far as implementation I have no clue (yet).  I have been looking
| through all of the tree evaluation stuff just trying to make sure I know
| what it does before I go tapdancing through the sources.
|
| Using the example above, if the invocation is gick(drk), then the Snork
| field of drk should be updated with the results of the gick function.  
| In general, the parent tree must be traversed to find the appropriate
| structure field for the class that owns the method and that structure
| field updated with the result of the method function.

OK, that's the kind of information I was looking for, not necessarily
exactly what the detailed implementation would be in Octave.  So who
is responsible for looking for parent classes in this case?  I'm not
sure I have a clear picture of what is supposed to be happening here,
and unfortunately I also don't have a lot of time to spend working on
this problem at the moment.

jwe