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

Add module name information to Call classes. #149

Open
lucidd opened this issue Apr 23, 2016 · 7 comments
Open

Add module name information to Call classes. #149

lucidd opened this issue Apr 23, 2016 · 7 comments

Comments

@lucidd
Copy link
Member

lucidd commented Apr 23, 2016

This will allow us to use the call objects to generate input for functions like state.high and state.low.

@admd
Copy link
Contributor

admd commented Feb 28, 2017

I would appreciate if you can provide a bit more information regarding this requirement.

@lucidd
Copy link
Member Author

lucidd commented Mar 1, 2017

The general idea of this was to have the *Call classes know about the module name of the function they are calling. There are multiple places where is would be helpful to know the module name. For example the state.high and state.low could potentially be used to to bundle multiple LocalCalls into one input for state.high/low via module.run. But to generate this input we need to know the modue name of each LocalCall we want to include.

@admd
Copy link
Contributor

admd commented Mar 1, 2017

Thanks @lucidd for the quick reply. While looking into this issue, i see that there are some duplication in *Call classes. Would it make sense to move the common code to an abstract class and extend our *Call classes from that? @renner

@admd
Copy link
Contributor

admd commented Mar 2, 2017

Ok back to the original issue. Two scenarios.
1st, keep the old constructor(for backward compatibility) where module name is appended to function like {modulename}.{function} and then get the module name from this parameter by some parsing rule.

2nd, add another constructor which accepts module name as a separate parameter to function.

Furthermore, function parameter can also be changed to an array of strings or varargs like {high, low} if want to pass more than one function. Also, instead of functions as Strings we can introduce Enums with all the functions of a module.

Pleas bear with me if am writing some not so intelligent stuff, i haven't get much chance to contribute in community before so ignore it if anything is rude or so. It's going to be my 1st PR in community project so i am way more conscious in my approach.

@lucidd
Copy link
Member Author

lucidd commented Mar 2, 2017

@admd if there is common code that can be put in a super class that would be very welcome. 👍

@renner
Copy link
Member

renner commented Jun 1, 2017

After the merge of #197 this issue is partly fixed: we now have the module information available, but we still would want to have a getter for only the function name. Note that the current getFunction() returns the whole string as composed from module.function.

@admd
Copy link
Contributor

admd commented Jun 1, 2017

I will take care of this on weekend .

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

No branches or pull requests

3 participants