Rename Classes

[markirish]

Current classes have names like 'iPgm' and 'iConn'. These names are bad for a number of reasons (arent Pgms and Conns but a class that calls programs and connections, don't fit in with with the style configuration of using uppercase classes/constructors).

We should create a 'new' API that renames these classes, and use that in our documentation. Then for the old class names, wrap the calls around the new calls and throw a warning that these calls are deprecated. This will help people using the old code transition, while also making sure new users are using the new API.

Classes that need to be fixed:

• iDataQueue
• iNetwork
• iObj
• iProd
• iConn
• iPgm
• iSql
• iUserSpace
• iWork

[markirish]

This also relates to #23

After spending more time in the code, it seems that there are two types of 'classes' in the nodejs-itoolkit project:

1. Classes that have a reason for being a class (e.g. iConn, ProgramCall)
2. Classes that exist only to organize functions, and are otherwise identical. These classes should probably be squashed into one file, as see how much code from them we can abstract out. These classes (and their functions) are:

• iDataQueue
  • sendToDataQueue
  • receiveFromDataQueue
  • clearDataQueue
• iNetwork
  • getTCPIPAttr
  • getNetInterfaceData
• iObj
  • retrUsrAuth
  • retCmdInfo
  • retrPgmInfo
  • retrSrvPgmInfo
  • retrUserInfo
  • retrUserAuthToObj
  • AddToLibraryList
• iProd
  • getPTFInfo
  • getProductInfo
  • getInstalledProducts
• iUserSpace
  • createUserSpace
  • setUserSpaceData
  • getUserSpaceData
  • deleteUserSpaceData
• iWork
  • getSysValue
  • getSysStatus
  • getSysStatusExt
  • getJobStatus
  • getJobInfo
  • getDataArea

Basically, all these functions create an output buffer, create a ProgramCall, add parameters, then call and manipulate the result into JSON with unique keys and structure based on the function. There is nothing 'Worky' about iWork, other than the functions it contains can be loosely grouped by what they accomplish.

Funnily, these files are the opposite of the files Abdirahim workers on for #20 and #28. In that case, it was the right call to split them into files. In this case, we should combine them into ONE file. So basically the code base as it existed last week was kind a negative of what I would think would be good coding principles.

To not break when going to v1.0, we will have to do the same thing we did with iPgm/ProgramCall, where we wrap the old around the new and throw a deprecation warning.

[markirish]

Ok, so here is the solution I have come up with:

Create a new class for iCmd, iQsh, and iSh. These were pulled out of itoolkit.js by @abmusse, but they really are just functions that all do the same thing, just with different 'flavors', and currently each in their own file. We should make a CommandGenerator class with 3 static functions, iCmd, iQsh, iSh. These could be changed to something like generateCmd() and likewise, but first we have to discuss CommandGenerator.

Per my previous post, I think we should combine iDataQueue, iNetwork, iObj, iProd, iUserSpace, and iWork into a Toolkit class (we can change the name, wasn't sure what to call it). So now instead of classes that don't serve a purpose other than to separate these functions, they are all part of Toolkit.

I also renamed the following classes:

iConn -> Connection
iSql -> SqlGenerator

The old class names are now deprecated, but still work. They now call the new classes/functions. The old code that routes to the new code can be found at the bottoms of the files that replaced them (so stubs of iConn.js are in the bottom of Connection.js)

So this is what I propose. I will make a merge request and we can discuss here or there.

Still not sure what to rename/do with these files: irest.js, istoredp.js, ixml.js.

[abmusse]

@markdirish

Create a new class for iCmd, iQsh, and iSh. These were pulled out of itoolkit.js by @abmusse, but they really are just functions that all do the same thing, just with different 'flavors', and currently each in their own file

To be clear the only purpose of #20 was to extract the classes and functions from itoolkit.js leaving it as our main entry point.

I agree that iCmd, iQsh, and, iSh are all essentially the same and could be contained within one common class. Good work further consolidating those files.

istoredp.js and irest.js are both transports will be consolidated to live in lib/transports in a future PR (when I get back to the office) see #26 . Leave those two files as is for now.

I added @kadler to further review PR #31 while I'm still away.

Can you provide examples of how you would create and use objects from the proposed classes:

• Toolkit.js
• CommandGenerator

[markirish]

Toolkit works the same as the old 6 classes: const toolkit = new Toolkit(conn);. That way the connection is held through various calls. If you want to add more options to the constructor, you only have to add them to Toolkit. Then to do something, do toolkit.getSysValue(value, () => { }); just like you would do with iWork.

For CommandGenerator, since it just has 3 static functions, we could just make a file and export 3 const functions instead of wrapping them in a class. Or we could add them to uutils.js As it is now, they would be called with CommandGenerator.generateQshXML(qsh, options). If we don't wrap in a class (which seems to be the real direction we should go), it would just be generateQshXML(qsh, options), or maybe it should be styled generateQshXml.

[kadler]

So, I think the point of the separate classes was to be more like IBM Toolkit for Java, aka jt400/jtopen. This has classes for most objects on the system and makes interaction very object oriented, eg. https://www.ibm.com/support/knowledgecenter/ssw_ibm_i_71/rzahh/javadoc/com/ibm/as400/access/DataQueue.html

So the question is, do we want this package to abstract XMLSERVICE at a high level and provide various classes to access resources in an object-oriented way or provide convenience APIs which generate the XMLSERVICE xml? Personally, I'm in favor of the former over the latter. I'd also be ok with having a separate package which does the high level abstractions.

[markirish]

Ah, if they mirror objects on the system then that would be a good reason to keep them separate. But right now they don't really allow for object-oriented interaction, more just convenience APIs that are grouped by object. So for the DataQueue example, you can sendToDataQueue, receiveFromDataQueue, and clearDataQueue, but you don't do these on an actual DataQueue object or anything that abstracts the system DataQueue object, you simply call them under an iDataQueue class that serves as a wrapper for API calls. The only data that is actually stored on the iDataQueue (or any of these classes) is the open connection.

So I guess since these 6 classes are already nothing more than creating "namespaces" for convenience APIs, we can group all of these together, then look at creating classes class Product or DataQueue or UserSpace that actually abstract an object on the system and provide ways to access those resources in an object oriented manner.

If I have grossly misunderstood anything, let me know.

[kadler]

Yes, I agree that as they are written they don't really fit. The question then becomes how do we want them to work and how do we maintain compatibility?

[abmusse]

Seems like this Package Generates input XML for XMLSERVICE for 3 main uses:

• Call a Program
• Call a Command (CL, QSH, or Shell command)
• Perform SQL operation

The rest of the functionality (iDataQueue, iNetwork, etc...) are wrappers over Program calls.

Currently we have:

• ProgramCall Class
• SqlCall Class

I think we should also have a CommandCall class

@markdirish You also mentioned something similar before

Create a new class for iCmd, iQsh, and iSh

The CommandCall Class could be pretty simple. Constructor accepts an object like:

{
  command: ''
  type: (cl, sh, or qsh)
  commandOptions: {..}
}

It would have toXml() similar to what ProgramCall and SqlCall currently have that returns the generated XML for the command. Within toXml() the proper function to generate XML for the specified command type would be called.

The rest of the utility functions (iDataQueue, iNetwork, etc..) currently reside on Toolkit Class as methods.

@kadler

For now compatibility is maintained by exporting wrappers for the old classes within Deprecated.js that call the renamed/new methods under the covers.

One compatibility issue is that current examples and users expect to require the utility functions from specific files within lib directory.

From the README

var wk = require('itoolkit/lib/iwork');
var nt = require('itoolkit/lib/inetwork')

Well those files do not exist anymore. Not sure how we can maintain compatibility with that.

We do expose all the pre v1.0 Classes/Methods from itoolkit.js

[abmusse]

Also current code uses console.warn() to display deprecation warnings within Deprecated.js.

After doing some research I think we should probably use the depd package to display our deprecation warnings.

Output would look something like this to end users when deprecated function is called:

[kadler]

That looks cool.

[markirish]

That does look really cool.