#396: streamlined Asset creation to be consistent for all asset types #398
Conversation
…r all asset types(native, alpha4, alpha12, pool share). Fixed TrustlineCreatedEffectReponse marshaling to work when asset is of pool share type.
…ctor from TrustlineCUDResponse.
| * @return Asset | ||
| */ | ||
| public static Asset create(String type, String code, String issuer, String liquidityPoolID) { | ||
| if (type == null) { |
There was a problem hiding this comment.
what's the rationale for changing the behavior of type to allow null values?
previously: type the type of asset can be 'native', 'alpha4', 'alpha12' or 'liquidity_pool_shares'
There was a problem hiding this comment.
horizon uses credit_alphanum4 and credit_alphanum12 instead of alpha4 and alpha12
There was a problem hiding this comment.
the alpha4, alpha12 type values were/are ignored, rather the asset code is introspected to determine the Asset class type, only type values of 'native' and 'liquidity_pool_shares' have effect on derived Asset class, otherwise, clients don't need to pass a specific 'alpha4, alpha12' type, for backwards compat, it's fine, if it's one of those values or null will result in the same introspection of asset code to determine Asset class.
There was a problem hiding this comment.
trying to stay with existing static factory method pattern, but one other creational pattern to consider, but thought it might be too much breaking change, is to remove the create methods, and replace with builder pattern, such as:
Asset.newBuilder().withCode().withIssuer().withLiquidityPoolId().create()
if you don't include any withXyz() then by default the builder creates a native.
checked for Asset representation(native, alpha4, alpha12, pool share) consistency for any API operation/effect resource that may have possibility of holding an Asset resource which refers to liquidity pool shares via
liquidity_pool_idand does not contain the usual non-native alpha4 or alpha12 attributes ofasset_codeandasset_issuer, these are the resources identified:TrustlineCreatedEffectReponse TrustlineUpdatedEffectResponse TrustlineRemovedEffectResponse ChangeTrustOperationResponseI refactored the Asset class to provide a bit more asset-type agnostic factory methods for generating Assets which derive the type(i.e. class model), so callers do not need to have same know-how.
closes #396