From maintainers-request at octave dot org Thu Nov 17 04:09:16 2005
Subject: Re: Octave_value class hierachy
From: Stefan van der Walt <stefan at sun dot ac dot za>
To: "John W. Eaton" <jwe at bevo dot che dot wisc dot edu>
Cc: octave-maintainers-list <octave-maintainers at bevo dot che dot wisc dot edu>
Date: Thu, 17 Nov 2005 12:12:06 +0200

John,

Since you're at it, can you please add one line comments to these
functions to indicate what they do?  When I read this code for the
first time I found it a bit daunting, and I think even very crude
comments will make it a lot simpler to understand.

Stéfan

On Thu, Nov 17, 2005 at 01:03:00AM -0500, John W. Eaton wrote:
> On 16-Nov-2005, John W. Eaton wrote:
> 
> | We would need to make the following changes:
> | 
> |   * octave_value::rep becomes a pointer to octave_base_value instead of
> |     octave_value
> | 
> |   * The octave_base_value class is no longer derived from octave_value.
> | 
> |   * All other value classes must be derived from octave_base_value (I
> |     think this is already true, but it would become a requirement).
> | 
> |   * Functions in the octave_value class would not need to be declared
> |     virtual, but the functions in octave_base_value would be virtual.
> | 
> |   * In addition to changing the type of octave_value::rep, we would
> |     also have
> | 
> |       typedef octave_base_value * (*type_conv_fcn) (const octave_value&);
> |       octave_value (octave_base_value *new_rep, int count = 1);
> |       octave_base_value *clone (void) const;
> |       octave_base_value *empty_clone (void) const;
> |       octave_base_value *try_narrowing_conversion (void);
> |       octave_base_value *internal_rep (void) const;
> | 
> |     in the octave_value class instead of
> | 
> |       typedef octave_value * (*type_conv_fcn) (const octave_value&);
> |       octave_value (octave_value *new_rep, int count = 1);
> |       virtual octave_value *clone (void) const;
> |       virtual octave_value *empty_clone (void) const;
> |       virtual octave_value *try_narrowing_conversion (void);
> |       octave_value *internal_rep (void) const;
> | 
> |     and the nil_rep function could be moved to the octave_base_value
> |     class.  These changes would affect all derived types, but should
> |     only require a change in the declaration, with no change in the
> |     way the functions actually work.
> | 
> |   * There would be no need for the octave_xvalue struct (which is just
> |     confusing anyway).
> | 
> |   * The rep could be assumed to always be non-null.
> | 
> |   * As Jens suggested, we could move count to the octave_base_value
> |     class.
> 
> OK, I spent some time making this change (not checked in yet; I may
> put it on a separate branch).  It turned up some bugs (for example,
> inappropriately casting an octave_value object to a derived type) and
> some questionable design (the numeric_conversion_function method, for
> one).