octave-maintainers
[Top][All Lists]
Advanced

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

Some strfns.cc loose ends


From: John W. Eaton
Subject: Some strfns.cc loose ends
Date: Wed, 25 Jul 2012 20:43:25 -0400

On 25-Jul-2012, Daniel J Sebald wrote:

| Given some of the conversations at OctConf, here are some questions to 
| resolve some loose ends in the strfns.cc string functions code.
| 
| As a summary, the strncmp and related functions will compare two arrays 
| of strings (or, I assume an array and a single string) via the array_op. 
|   If an argument is a cell, then there is more tests and work for 
| various scenarios.  For example, there is a test:
| 
|                    if (cell_val.is_cellstr ())
| 
| Recall our discussion at OctConf where we wondered about the speed of 
| iscellstr.  Is the is_cellstr() above related to that?  In other words 
| is there a possible speed issue here?
| 
| Also recall the solution was a flag variable associated with the cell, 
| or as proposed by Max caching the type in some cases.
| 
| Now, look at the way the
| 
|                        // FIXME: should we warn here?
|                        for (octave_idx_type i = 0; i < r; i++)
|                          {
|                            if (cell(i).is_string ())
|                              output(i) = str_op (str[i], 
| cell(i).string_value (), n);
|                          }
| 
| If there is a nonstring in the cell, then Octave will just skip over the 
| comparison.  That is why there is a FIXME, because that really doesn't 
| feel like the right thing to do.  I think this line
| 
|            boolNDArray output (cell_val.dims (), false);
| 
| means that output is predefined as false (i.e., are not the same), so 
| there is no uncertainty about skipping over the test.  But still, it 
| seems someone uncomforting to NOT issue an error.
| 
| So what do people think about just iterating through the array and 
| testing whether the element is a string.  If not, issue an error and 
| break to the command line?  That way, we could leave out the 
| cell_val.is_cellstr() member function test if it is slow.  (Or, if we 
| plan to optimize is_cellstr() for speed in the future it could be left in.)
| 
| In any case, thoughts on getting rid of the FIXME by simply error-ing 
| out when the element is a non-string as opposed to skipping and 
| indicating 0?

If you want compatibility with Matlab you can't produce an error
here.  Complete compatibility means no warning either.  But at least
with a warning with an ID, the user has the choice of disabling it or
turning it into an error.

jwe


reply via email to

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