astroband / ruby-stellar-base

The stellar-base library is the lowest-level stellar helper library. It consists of classes to read, write, hash, and sign the xdr structures that are used in stellar-core
Apache License 2.0
22 stars 19 forks source link

Provide alternative parameters types for amount, asset #53

Closed JakeUrban closed 4 years ago

JakeUrban commented 4 years ago

Originally discussed here, the following adjustments should be made to all Operation methods accepting amount or asset parameters:

These changes are backwards compatible. All examples will be changed to use the new parameter types.

JakeUrban commented 4 years ago

An issue I've encountered is that pay_payment methods require a :with attribute, which has the same type as the usual :amount attribute.

This means we'd need 2 asset parameters added, one for the destination asset and another for the sending asset. It also means we'd need to add a with_amount parameter, so users of the new parameters would ultimately pass:

And the code would still need to support the original :with and :amount types.

The above situation makes me second-guess the value of making this change. My original intention was to make the parameters more intuitive since the current situation of passing an array of values that are used to construct amount and asset types seemed messy. But this situation has helped me see why it would makes sense to pair the amount with the asset and use one parameter.

So if we want to continue to have this tight coupling between an asset and its amount, I think the better solution is to accept the original types OR an array of type [Asset, String or Numeric].

This is nice since we don't have to add additional parameters to these methods, and it still has the tight coupling between an asset and its amount. It just replaces the pattern of passing the asset type, code, and issuer instead of passing the Asset object containing the same information.