Re: io-2.4.13 released

[Andrew Janke]

On 10/17/19 12:51 PM, Philip Nienhuis wrote:
> Hi,
> [...]
>
> [...] I do want to adapt the spreadsheet I/O 
> functions code base to a much more easily maintainable status. A benefit 
> is that it'll make it easier to move them to core some day, maybe.
>
> An important part is merging the now largely separate ods????? and 
> xls????? functions sets into just one xls????? function set and morphing 
> the ods????? functions into wrappers for their xls????? counterparts and 
> deprecate them. After all, Matlab's xlsread can perfectly read .ods 
> these days and we do want to be compatible.
> This way a lot of duplicate code can be eliminated.

I like this idea of merging them, especially if the file type could be 
auto-detected. One less thing for the user to worry about.

> A next step might be splitting up the OCT interface .xlsx and maybe .ods 
> private functions into smaller dedicated pieces. However, .xlsx and .ods 
> are very complicated file formats so maintaining these functions still 
> won't be easy peasy.

Here's an idea that you may or may not like: how about refactoring the 
internal implementation into Octave OOP classdef classes? The pattern of 
having "COM_", "JOD_", "JXL_", "POI_", etc variants of the same set of 
"__XXX_chk_sprt__", "__XXX_spsh_open" etc functions, which are called 
inside switch statements, just screams "polymorphism" to me. And then 
there's the XLS vs XLSX vs ODS axis. Turning those into a family of 
classes like "ComExcelAdapter", "PoiExcelAdapter", "UnoOdsAdapter" might 
make organizing the code easier, and less painful to split up into small 
methods. Then a bunch of OCT interface methods could live inside their 
class without cluttering up the shared function namespace and making the 
inst/ directory really big.

OOP could also be a nice way of representing more complicated state like 
cell formatting controls, style definitions, and so on. And the "file 
handle" structs returned by xlsopen() and odsopen() could be replaced 
with better-typed objects. Two "assert (isa (...), ...)" calls would 
replace this verbose, approximate type checking:

  ## Check if xls struct pointer seems valid
  if (! isstruct (xls))
    error ("File ptr struct expected for arg @ 1\n");
  endif
  test1 = ! isfield (xls, "xtype");
  test1 = test1 || ~isfield (xls, "workbook");
  test1 = test1 || isempty (xls.workbook);
  test1 = test1 || isempty (xls.app);
  test1 = test1 || ~ischar (xls.xtype);
  if test1
    error ("Invalid xls file pointer struct\n");
  endif

  ## Check worksheet ptr
  if (! (ischar (wsh) || isnumeric (wsh)))
    error ("Integer (index) or text (wsh name) expected for arg # 2\n");
  elseif (isempty (wsh))
    wsh = 1;
  endif


And then you could use Octave's handle class's delete() destructor to do 
garbage collection on resources that need to be cleaned up, like active 
COM Excel or UNO LibreOffice sessions running in the background, the 
opened workbook files, and so on.

Stick them in a "forge.io.internal" namespace to indicate that they're 
for Octave's internal use and end user code should use them directly.

I realize this would be a big change, though.

Cheers,
Andrew