Re: general c++ question

[Shai Ayal]

On 8/29/07, John W. Eaton <address@hidden> wrote:
> On 29-Aug-2007, Shai Ayal wrote:
>
> | The one thing I didn't like at first is the set_XXX method:
> | Until this morning I though it would be best not to allow direct
> | change of properties not through the normal set(h,name,val) functions.
> | This ensures that properties are checked for errors, and while it does
> | carry a performance hit over direct access, the graphics backend does
> | not need to set many values. However, this morning I remembered that
> | thery are some  properties which are "read-only" -- informative
> | properties such as mouse position etc... The easiest way to implement
> | read-only properties is not to add them into the XXX::set function,
> | but to add them to the XXX::get function. This dictates that the
> | backend will have direct access to setting the methods not through the
> | set function.
>
> OK, I wasn't thinking about these kinds of things, and the set_X
> functions are not really necessary for the rest of the patch to work.
> But maybe we should make some additional changes.
>
> Currently we have (for example):
>
>   void
>   axes::axes_properties::set (const property_name& name,
>                               const octave_value& val)
>   {
>     bool modified = true;
>
>     if (name.compare ("parent"))
>       set_parent (val);
>     else if (name.compare ("children"))
>       children = maybe_set_children (children, val);
>     else if (name.compare ("__modified__"))
>       {
>         __modified__ = val.bool_value ();
>         modified = false;
>       }
>     else if (name.compare ("position"))
>       position = val;
>     else if (name.compare ("title"))
>       {
>         graphics_handle h = ::reparent (val, "set", "title",
>                                         __myhandle__, false);
>         if (! error_state)
>           {
>             if (! xisnan (title))
>               gh_manager::free (title);
>
>             title = h;
>           }
>       }
>     ...
>
> and this ensures some consistency, handles the special cases in which
> setting some properties implies changes to other properties, etc.
>
> Maybe we should rewrite this as simply
>
>   void
>   axes::axes_properties::set (const property_name& name,
>                               const octave_value& val)
>   {
>     bool modified = true;
>
>     ...
>     else if (name.compare ("position"))
>       set_position (val);
>     else if (name.compare ("title"))
>       set_title (val);
>     ...
>
> and have the individial set_X functions take care of handling the
> dependent properties and consistency checks?

This way you will loose the easiness of having them automatically
generated in the macro. But it is better and more consistent.

I hate these cut & paste patches. If no-one else volunteers, I'll do
it, but it's not really a high priority with me so it will take me
some time.

Shai
> There may still be some special cases (I think at least the
> __modified__ property).
>
> jwe
>