Skip to content
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

Programs & service programs #5

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open

Programs & service programs #5

wants to merge 34 commits into from

Conversation

jordiwes
Copy link
Collaborator

Please Marco the code

def test_rainbow(self):
int = Integer("aint8", 3, 1)
float = Float("afloat", 4, 2, 5.55)
char = Character("achar", 32, "A")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int and float are built-in Python functions, which you've now just re-defined. Might want to use different variable names here.

pykit/types.py Outdated
self.name = str(name)
self.isReturn = False
self.byValue = False
self.payload = {"name":self.name}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should all be prefixed with __ to ensure they are private only to the class/subclass. This will break ServiceProgram.add_return(), though.

self.name = name
self.library = library
self.parameters = []
self.payload = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these are going to be private fields, we should think about indicating as so with a preceding single or double underscore: https://docs.python.org/3/tutorial/classes.html#private-variables

Not something to hold this up, but something to think about.

"""
Object for IBM i Decimal.
"""
def __init__(self, name, length, decimals, value, signed=False):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, apparently indeed the s does stand for "signed". There's also an "l" and "r" types which have a leading or trailing sign digit separate from the numeric data (I wonder if anyone actually uses those?). I don't know of anybody that refers to them as "signed decimal" though (could be showing ignorance here) - I've always heard it referred to as zoned decimal. Indeed the the free-format names use ZONED and PACKED. Perhaps instead, we just flip the condition and use packed=True?

Alternatively instead of a parameter, use separate classes: ZonedDecimal and PackedDecimal (or Numeric and Decimal) You could use this class as an internal base class.

"""
Object for IBM i Float.
"""
def __init__(self, name, length, precision, value):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can a float have a precision? I think it has to be specified as 0. If so, just like the Integer class, leave it off the parameter list.

@@ -0,0 +1,157 @@
class Parameter:
def __init__(self, parameterType, direction='inout', byValue=False, isReturn=False):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way dbsock handles return values is weird and unintuitive. A return value by definition cannot be an input, so why does it have a direction? I think there should have been a separate way to specify the return value instead of trying to shoe-horn it in as a "parameter".

I'd like to see us hide this quirk as much as possible. Can we split out the return value handling in to a separate class, maybe ReturnValue and then add a new method add_return?

"""
Object for IBM i Character.
"""
def __init__(self, name, length, value, varying=''):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a separate VarChar class instead of a parameter?

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 this pull request may close these issues.

None yet

3 participants