[Octave-bug-tracker] [bug #65244] Simplify code complexity of perms.cc

[Hendrik K]

Follow-up Comment #5, bug#65244 (group octave):

Thanks for the review.

I think that declaring myvidx as octave_idx_type would (slightly) improve the
robustness of the code without much harm done in terms of used memory space:


In practical terms, the number of permutations explodes extremely quickly, so
everything beyond myvidx of 12 will use all the available computer memory
(e.g. myvidx = 15 would require hundreds of terabytes of memory)

Therefore I did not even think about anything beyond an "int" or even an
"int8" for myvidx...

But through the parameter "unique", one could come up with (one) extreme case:
a very long vector of identical values (size larger than an int) for which the
perms solution with "unique" is the vector of identical values itself. This
would not work with the current implementation. 
Note that if there are two non-identical values in such a long vector beyond
intmax values, the result would explode beforehand.


So changing myvidx to octave_idx_type would (slightly) increase the robustness
but also especially the logical soundness/clarity of the code: 

When one gets an Octave array, one should expect a maximum index fitting in an
octave_idx_type variable and therefore use octave_idx_type for index variables
for Octave input arrays. This is already done in the code.

But this should also be true for interim/dependent variables which are
directly driven by the size of the input array (the case of myvidx we are
looking at here)  

So good catch...



    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?65244>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/