Re: Graphics properties as objects

[John W. Eaton]

On  1-Jan-2008, Michael Goffioul wrote:

| On 12/30/07, John W. Eaton <address@hidden> wrote:
| > Who is responsible for creating and deleting rep?
| 
| Here's a more detailed version of what I had in mind. It's far from
| finalized, but it should give you the main idea. The first part of the
| file contains the code for properties, the second part shows how this
| code is intended to be used in the graphics code.
| 
| Happy new year.
| 
| Michael.
| class property;
| 
| class base_property
| {
| public:
|   friend class property;
| 
| public:
|   base_property (const std::string& s, const graphics_handle& h)
|     : count (0), name (s), parent (h), hidden (false) { }
| 
|   virtual ~base_property (void) { }
| 
|   std::string get_name (void) const { return name; }
| 
|   graphics_handle get_parent (void) const { return parent; }
| 
|   bool is_hidden (void) const { return hidden; }
| 
|   void set_hidden (bool flag) { hidden = flag; }
| 
|   virtual void set (const octave_value& val)
|     { error ("set: invalid property \"%s\"", name.c_str ()); }
| 
|   virtual octave_value get (void) const
|     {
|       error ("get: invalid property \"%s\"", name.c_str ());
|       return octave_value ();
|     }
| 
|   base_property& operator = (const octave_value& val)
|     {
|       set (val);
|       return *this;
|     }
| 
| private:
|   int count;
|   std::string name;
|   graphics_handle parent;
|   bool hidden;
| 
| private:
|   base_property (const base_property& bp) { }
| };
| 
| class string_property : public base_property
| {
| public:
|   string_property (const std::string& s, const graphics_handle& h,
|                    const std::string& val = "")
|     : base_property (s, h), str (val) { }
| 
|   void set (const octave_value& val)
|     {
|       if (val.is_string ())
|         str = val.string_value ();
|       else
|         error ("set: invalid string property value for \"%s\"", get_name 
().c_str ());
|     }
| 
|   octave_value get (void) const
|     { return octave_value (str); }
| 
|   std::string string_value (void) const { return str; }
| 
| private:
|   std::string str;
| 
| private:
|   string_property (const string_property& p) { }
| };
| 
| class property
| {
| public:
|   property (base_property *bp) : rep (bp)
|     { rep->count++; }
| 
|   property (const property& p)
|     {
|       rep = p.rep;
|       rep->count++;
|     }
| 
|   ~property (void)
|     {
|       if (--rep->count <= 0)
|         delete rep;
|     }
| 
|   std::string get_name (void) const
|     { return rep->get_name (); }
| 
|   graphics_handle get_parent (void) const
|     { return rep->get_parent (); }
| 
|   bool is_hidden (void) const
|     { return rep->is_hidden (); }
| 
|   void set_hidden (bool flag)
|     { rep->set_hidden (flag); }
| 
|   property& operator = (const octave_value& val)
|     {
|       *rep = val;
|       return *this;
|     }
| 
|   property& operator = (const property& p)
|     {
|       rep = p.rep;
|       rep->count++;
|       return *this;
|     }
| 
|   string_property& as_string_property (void) const
|     { return *(reinterpret_cast<string_property*> (rep)); }

Since this is  casting within the inheritence hierarchy, shouldn't
this use dynamic_cast?

Also, should it really return a non-const reference?  Since rep is
shared, wouldn't that allow someone to modify multiple copies?  Is
that really what you want here?

| >>>>> expands to <<<<<
| 
|   property string;
| 
|   string_property& get_string (void) const { return string.as_string_property 
(); }
| 
|   void set_string (const octave_value& val)
|     {
|       if (! error_state)
|         {
|           string = val;
|           mark_modified ();
|         }
|     }
| 
| >>>>> property creation <<<<<
| 
| text::properties::properties (const graphics_handle& mh,
|                             const graphics_handle& p)
|   : base_properties (go_name, mh, p),
|     string (new string_property ("string", mh, "")),
|     //...
| {
|   //...
| }

OK, now that I've looked at the code, I think your proposal is OK in
that it still provides the specific get_X functions.

| >>>>> property set in base_properties <<<<<
| 
| class OCTINTERP_API base_properties
| {
| //...
| public:
|   void add (base_property *bp)
|     { all_props[bp->get_name ()] = property (bp); }

Where is this function used?

| >>>>> then the easiest is probably to rewrite properties ctor <<<<<
| 
| text::properties::properties (const graphics_handle& mh,
|                             const graphics_handle& p)
|   : base_properties (go_name, mh, p),
|     string (new string_property ("string", mh, "")),
|     //...
| {
|   //...
|   add (string);
| }

OK, I think this is a reasonable solution.

jwe