search

voith / eth-tester-rpc

Rewrite of piper merriams eth-testrpc
MIT License
3 stars 3 forks source link

Replace lambda pattern with curry #22

Closed pipermerriam closed 6 years ago

pipermerriam commented 6 years ago

What was wrong?

https://github.com/voith/eth-tester-rpc/blob/2392b3716c16e22804caa8adf5a9b8102fec4ec2/eth_tester_rpc/rpc.py#L390-L395

In the main RPCMethods class a lamba is used to return a function which can then be called with the JSON-RPC request args to return the API result. While lambda functions are fine, it could be a little cleaner which might be worth it (not definitive)

How can it be fixed?

I believe that this can be cleaned up using functools.partial.

If the line above was instead, return functools.partial(call_delegator, delegator, self.client, method) I believe it would be functionally equivalent with the benefit of having cleaner stack traces.

This can potentially be done with cytoolz.curry though I think the result would be less clean due to the need for *args style calling, which can be done as follows.

def call_delegator(delegator, client, method):
    def _call_delegator(*args):
        ... # body here
    return _call_delegator

It's a bit fugly with the inlined function definition...

voith commented 6 years ago

Thanks, @pipermerriam for taking the time to review the code. I will definitely incorporate your suggestion.

Just FYI, I wanted to contribute this to eth-tester but I was encouraged by @carver to maintain it here. I will be integrating this in populus as I had removed the old testrpc chain from populus in https://github.com/ethereum/populus/pull/474.

pipermerriam commented 6 years ago

I could see this making its way into eth-tester at some point though it would probably be something that lived under extras_require since we don't want to push the extra dependencies onto the base library. Lets give it some time to mature and then we can look into a merger if you are still interested and it still seems like a good idea.