octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: obscure code features?


From: John W. Eaton
Subject: Re: obscure code features?
Date: Thu, 24 Jun 2010 18:01:41 -0400

On 24-Jun-2010, David Bateman wrote:

| Jaroslav Hajek wrote:
| > hi all,
| >
| > I'm asking about a couple of particular features. Perhaps someone
| > (John, David) will be able to answer at least some of the questions
| >
| > 1. A number of octave_value extractors take a bool parameter
| > frc_str_conv (e.g. double_value). Certain other extractors don't (such
| > as int8_value).
| > Except forwarding, it doesn't seem to be actually used anywhere. What
| > the heck is/was it good for?
| >   
| I think this is just cruft that is left-over from when these extractors 
| had  a boolean parameter, in most cases to suppress error messages 
| during type conversions. The arguments seem to have been kept to 
| preserve the API, though are no longer used. As far as I can see they 
| have been used for a long time and most be probably be deleted.

I think this is correct and I agree that the function paramters can be
deleted.

| > 2. convert_to_str, convert_to_str_internal - what are these for? Can't
| > string_value/char_matrix_value be used?
| >   
| 
| This stuff all pre-dates when I started hacking Octave. I just lived 
| with it.

I don't remember the reason for having these.  Maybe to avoid
warnings?  In any case, if they are no longer needed, then removing
them is OK with me.
| 
| > 3. why does octave_char_matrix exist at all? (I think I might have
| > already asked this before, but I'm not sure)
| >   
| I thought the octave_char_matrix existed, to do whitespace filling at 
| the end of lines for cases like ["Short String", "Very Very Very Very 
| Long String"], as that is what the other brand does. This is convenient 
| to do in an array type.

We have octave_char_matrix which would have been used as numeric type
the same width as the C "char" type, but it was never used.  Now we
have int8, so I don't see the need to keep it.  The derived type
octave_char_matrix_str was for the character matrix type.  I'd say
merge the two classes and then replace octave_char_matrix_str with a
typedef.

| > 4. The pad parameter to all_strings is not used anywhere. What is/was
| > it for? Can it be removed? Without that, all_strings could simply call
| > cellstr_value.
| >   
|  Not sure, but it might have been related to point 3 in the passed.

Yes, it was probably related to padding character strings.  If that is
always done now and the parameter is not needed, then I don't object
to removing it.  We just need to be careful when removing parameters
that have default values.  For example, if we currently have something
like

  void foo (bool x = true, bool y = true);

and we remove X from the parameter list and someone has code like

  foo (false);

we have just introduced a bug...

This shouldn't be a problem with the all_strings function since it
only has one parameter, but it could be trouble for pad where it is
the first argument.  OTOH, if you are removing convert_to_str instead
of just removing the PAD argument, we should be OK, except that anyone
using the function will probably not be happy.  OTTH, we never
promised a stable API, but it might be good to note these kinds of
changes in the NEWS file and to provide a suggested replacement for
people who were using the obsolete functions.  Or, maybe we should
just mark these functions as deprecated for a release or two before
they are removed?

jwe


reply via email to

[Prev in Thread] Current Thread [Next in Thread]