[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