Web3.swift icon indicating copy to clipboard operation
Web3.swift copied to clipboard

Invoking contract methods using a [ABIEncodable] alongside ABIEncodable... parameters

Open Nixsm opened this issue 6 years ago • 4 comments

Problem

Currently we have this method to invoke methods of a contract:

public subscript(_ name: String) -> ((ABIEncodable...) -> SolidityInvocation)? {
    return methods[name]?.invoke
}

However, variadic functions a bit of pain to work with, in a sense that we can't send an array of parameters, we have to type each parameter separated by commas.

Possible Solution

I suggest we drop the variadic parameter and create a subscript method that receives an array of ABIEncodable and then, create a normal method to be used with variadic parameters, something like this:

// New subscript method
public subscript(_ name: String) -> (([ABIEncodable]) -> SolidityInvocation)? {
    return methods[name]?.invoke
}

public func invokeMethod(with name: String) -> ((ABIEncodable...) -> SolidityInvocation)? {
    return methods[name]?.invoke
}

Of course we'd need to refactor the SolidityInvocation implementations.

Disclaimer

  • Mentioned this at #49
  • I'd like to hear your opinion on the topic, I'd be down to implement everything proposed here

Nixsm avatar Jul 11 '18 13:07 Nixsm

I think this should be fine. Is there a reason why we currently use a variadic parameter instead of an array? Just convenience? @pixelmatrix

koraykoska avatar Jul 22 '18 22:07 koraykoska

I was just trying to emulate a natural way to call these so it actually looks like a function. I totally agree that variadics are a pain to work with if you're not calling them directly though. I think it just depends what we want the default usage to be.

myDynamicContract['getBalance']?(myAddress)

or…

myDynamicContract['getBalance']?([myAddress])
// alternatively…
myDynamicContract.invokeMethod(with: 'getBalance')?(myAddress)

What I was really aiming for though was to support dynamic member subscripts once Swift 4.2 is released so we could do:

myDynamicContract.getBalance?(myAccount)

pixelmatrix avatar Jul 23 '18 18:07 pixelmatrix

I'll implement this asap and send a PR, thanks for the response guys!

Nixsm avatar Jul 23 '18 20:07 Nixsm

I'll leave the default subscript method and add a new invokeMethod to the contract, so we keep the variadic interface working

Nixsm avatar Aug 06 '18 21:08 Nixsm