Re: nfields replaced by numfields on default branch

[Carnë Draug]

On 3 March 2014 15:10, Rik <address@hidden> wrote:
> On 03/03/2014 12:36 AM, Lukas Reichlin wrote:
>>
>> On 03.03.2014, at 09:25, Lukas Reichlin <address@hidden> wrote:
>>
>>> On 02.03.2014, at 22:01, Rik <address@hidden> wrote:
>>>
>>>> 3/2/14
>>>>
>>>> All,
>>>>
>>>> I deprecated nfields and added the new function numfields (in analogy with
>>>> numel) to replace it.  This probably won't make a difference to most people
>>>> since it was under-utilized.  I changed some core m-files to now use
>>>> numfields which made the code cleaner and simpler.  The changeset is here
>>>> (http://hg.savannah.gnu.org/hgweb/octave/rev/fcd87f68af4f).
>>>>
>>>> --Rik
>>>
>>> I don't quite get it. First you want to delete nfields, a function 
>>> introduced in Octave 3.4 (!) because it is not widely used (yet). Then you 
>>> rename it to numfields in analogy to numel. What about ndims or nnz? Should 
>>> it be "numdims" or "numnz" then as well?
>>>
>
> Dan and jwe convinced me that structs, as first-class objects like
> matrices, really should have their own way of determining the number of
> elements.
>
> Matlab already has numel, ndims, and nnz so there is no possibility for
> renaming those and staying compatible.  On the other hand, The Mathworks
> does not offer a numfields function so we are free to make our own,
> hopefully smart, naming decision.

But is numfields really the number of elements? The number of elements
would be "numfields (s) * numel (s)". And maybe even minus elements
that are may be empty is some of the scalar structs but not all.

structs are first-class objects but are a class where the number of
elements does not really matter. Their field names and the number of
structs matter, not the number of fields (unless it is zero). And this
can be seen easily by the fact that every single use of nfields until
now only checks if it returns zero. If anything, we'd need a function
that can show if the struct has any field. That would be much more
useful. If the only use of this function is to replace:

isempty (fieldnames (s))

with

nfields (s) == 0

we better replace it with

hasfields (s)

or something like that.

>>> The reason why I object to your change is that I want to keep the control 
>>> package's dependency on Octave's version number as low as possible. 
>>> Currently it's 3.6.0 and I don't want to raise it for no obvious reason.
>>>
>>> In order to not exclude users (e.g. Debian and Ubuntu), I would have to 
>>> create a DEFUN_DLD "nfields2" or the like such that there are no conflicts 
>>> in Octave 3.6 and 3.8 as well as 4.0 and later. Then I'd have to keep 
>>> oct-file "nfields2" in the control package until some show-stopping bug or 
>>> important feature requires me to raise the dependency to Octave 4.0 or 
>>> later.
>>>
>
> It's simple to write code that is portable to 3.0 and 4.0.  Just use
>
> number_of_fields = numel (fieldnames (obj));
>
> This doesn't depend on any internal Octave function and is the only way one
> has to do it in Matlab.
>
> I only deprecated nfields in 4.2 which means there is still years before
> you need to make any changes to code or versioning requirements.

Which also means that until 4.2 comes out, more people may start using
nfields. Is renaming nfields to numfields really worth causing that
trouble to others? The name nfields is pretty clear on its purpose. I
still think we should remove the function from Octave core.

Finally, if anyone things nfields is an interesting function but not
really worth of filling Octave's core workspace, why not move it to
the struct package? If this type of functions are not fitting to that
package, what is? And the function, in C++, is small enough to not be
a burden to anyone:

DEFUN (numfields, args, ,
       "-*- texinfo -*-\n\
@deftypefn {Built-in Function} {} numfields (@var{s})\n\
Return the number of fields of the structure @var{s}.\n\
@seealso{fieldnames}\n\
@end deftypefn")
{
  octave_value retval;

  if (args.length () == 1 && args(0).is_map ())
    {
      retval = static_cast<double> (args(0).nfields ());
    }
  else
    print_usage ();

  return retval;
}

Carnë