Skip to content

refactor: Add Internal Function to handle adding Data Structure Parameters #107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
abmusse opened this issue Feb 11, 2020 · 5 comments · Fixed by #139
Closed

refactor: Add Internal Function to handle adding Data Structure Parameters #107

abmusse opened this issue Feb 11, 2020 · 5 comments · Fixed by #139
Milestone

Comments

@abmusse
Copy link
Member

abmusse commented Feb 11, 2020

Looks like ProgramCall.addParam Could be simplified by creating another function that handles adding data structure parameters.

That way we could avoid the recursive calls to addParam.

This is out the scope of this PR therefore I will open a separate issue to track this.

Originally posted by @abmusse in #104 (comment)

@abmusse abmusse changed the title refactor: ProgramCall.addParam to avoid recursive calls. refactor: ProgramCall.addParam to avoid recursive calls Feb 11, 2020
@abmusse abmusse added this to the Version 1.0 milestone Feb 21, 2020
@kadler
Copy link
Member

kadler commented Feb 21, 2020

I would also argue that the entire function needs to be redesigned with a better user experience. Here's our current example of calling it: https://github.com/IBM/nodejs-itoolkit#example

Some of the problems:

  • There's a last parameter that is not supposed to be used by developers, but only internally (inDs).
  • If you want to pass a DS, you pass an array of fields in the first argument and options comes second, but if you want to pass a scalar value, you have to pass 3 arguments: the value, type, and options.
  • The type and value arguments are flipped around. Type should come first, then value.
  • No ability to specify a name for the arguments or structure fields.

Additionally, default values should not need to be specified. XMLSERVICE should be able to figure out a reasonable default and if not, itoolkit should be able to instead.

@kadler
Copy link
Member

kadler commented Feb 21, 2020

I have some ideas for how this should work, I'll detail them soon.

@kadler
Copy link
Member

kadler commented Feb 26, 2020

Compare the existing example to a re-written version.

const program = new ProgramCall('QWCRSVAL', { lib: 'QSYS' });

const outBuf = [
  // for code density, fields can be defined as arrays
  // [ name, type, value (optional), options (optional) ]
  // [ type, value (optional), options (optional) ]
  // (disambiguate between name/type/value and type/value/options by type of
  //  third arg: if object it's type/value/options)
  ['sysvals_returned', '10i0'],
  ['offset_to_data', '10i0'],
  // this parameter we don't care about, so we can leave it nameless
  ['36h'],

  // data structures can be nested. In that case, value must be undefined
  ['system_value_information', [
      ['sysval', '10A'],
      ['data_type', '1A'],
      ['data_status', '1A'],
      ['data_length', '10i0'],
      ['data', '10i0'],
  ]],
];

const errno = [
  // fields can be defined using objects as well
  // attributes are set on the object itself, not in a separate object
  {name: 'bytes_provided', type:'10i0', setlen: 'rec2' },
  
  {name: 'bytes_available', type: '10i0'},

  // The value can be set on the object
  {name: 'msgid', type: '7a', value: 'XXX0000'},

  // we can also omit names in object format 
  {type: '1h'},

  // Data structures can also be nested using object format
  {name: 'message fields', type='ds', fields=[
      // You can always mix and match object/array fields
      ['field1', '10i0'],
      {name: 'field2', type:'10i0'}
  ]}
];

// Same disambiguation as above
// addParam(name, type, value (optional), options (optional))
// addParam(type, value (optional), options (optional))
// addParam(options)

program.addParam('outbuf', outBuf, { io: 'out', len: 'outbuf_size' });
program.addParam({name: 'outbuf_len', type:'10i0', setlen: 'outbuf_size'})
program.addParam('retrieve_count', '10i0', 1);

program.addParam({name: 'sysvals', type:'10A', value:'QCCSID'});

program.addParam(errno, { io: 'both', len: 'rec2' });

There are a variety of options here, though I don't know that all would be necessary to support. Certainly, iPgm would have to support the existing interface and convert it's format to ProgramCall's format.

@abmusse
Copy link
Member Author

abmusse commented Mar 3, 2020

I think using this syntax where a single object is passed makes sense.

program.addParam({name: 'outbuf_len', type:'10i0', setlen: 'outbuf_size'})

Its flexiable and we could can convert to this format relatively easily in iPgm

The main downside to this approach is that the code will could get really wide.

@abmusse abmusse changed the title refactor: ProgramCall.addParam to avoid recursive calls refactor: Add Internal Function to handle adding Data Structure Parameters Mar 10, 2020
@abmusse
Copy link
Member Author

abmusse commented Mar 10, 2020

Looks like ProgramCall.addParam Could be simplified by creating another function that handles adding data structure parameters.

That way we could avoid the recursive calls to addParam.

This is out the scope of this PR therefore I will open a separate issue to track this.

Originally posted by @abmusse in #104 (comment)

After realizing that a DS can be nested within another DS I see why recursion was used initially.

I still think we should have a separate internal function to handle adding a DS.
Then within ProgramCall.addParam we can call that function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants