elucidsoft / dotnet-stellar-sdk

Stellar API SDK for .NET 6.x
Apache License 2.0
117 stars 55 forks source link

Alphanumeric assets are more specifically typed. #377

Closed GuyWhoKnowsTheGuy closed 1 year ago

GuyWhoKnowsTheGuy commented 1 year ago

Types of changes

These changes introduce a breaking change for the sake of (1) fixing a design flaw and (2) adding convenience when accessing an asset's code or issuer. The biggest issue, imho, is the fact that CreateNonNativeAsset orders the "Code" parameter before the "Issuer" parameter. The overloaded method that was removed, and likely used out in the field, orders the "Issuer" parameter before the "Code" parameter. If one simply removes the "AssetType" parameter, they will get runtime errors. However, if they have any unit tests with alphanum assets, it will be caught each and every time, since the an issuer is always more than 12 characters long. All in all, I believe this breaking change does far more good than harm, and will almost certainly be caught on first test/debug run.

elucidsoft commented 1 year ago

I think it may make sense to include this in the .NET 6 branch since that's also a breaking change. Thoughts?

GuyWhoKnowsTheGuy commented 1 year ago

Agreed, these changes have been put in the .NET 6 branch.