Boxes: Add convenience methods for encoding Box names

[michaeldiamant]

Adds convenience methods to ensure proper Box name encoding when querying Boxes.

Details:

• Since com.algorand.algosdk.v2.client.common.AlgodClient is auto-generated, it's not possible to provide manual extensions (e.g. expose another route). So, the PR opts to provide utility methods that can be used by the library user.
• PR defines BoxQueryEncoding in util package following the repo's prevailing convenience method location.

[algochoi]

How do we feel about supplying this method as a member in AppBoxReference instead of its own class? I think it could be nice if users could do something like

AppBoxReference abr = AppBoxReference(123, someBytes);
// Submit transaction with abr
Response<Box> boxResp = clients.v2Client.GetApplicationBoxByName(appId).name(abr.queryEncoding()).execute();

where the member method can convert the box object's byte name to a goal app-arg encoding.

[michaeldiamant]

How do we feel about supplying this method as a member in AppBoxReference instead of its own class?

@algochoi It's a reasonable question. @algoidurovic raised a similar question in go-algorand-sdk.

• Here's my original thinking: Boxes: Add convenience methods for encoding Box names go-algorand-sdk#342 (comment).
• Since you've both provided similar feedback, I'm on-board to make the change for AppBoxReference and BoxReference.
• Do you agree with retaining Box's convenience method as is?
  • I'd like to agree on the format to minimize change cycles. I can discuss live if helpful.

[algochoi]

Do you agree with retaining Box's convenience method as is?
I'd like to agree on the format to minimize change cycles. I can discuss live if helpful.

@michaeldiamant
I agree that Box (the generated model) should not be changed. I don't think a change to the generator for this would be necessary.

I think only the AppBoxReference would probably need the utility function, as users likely won't interact with the encoded version (with foreign array index) anyway.

[michaeldiamant]

@algochoi e05aa04 applies the feedback - let me know what you think.

A point of context: Generally, I view moving serialization/encoding into a domain object as a lack of separation of concerns. Since preferred alternatives (e.g. defining an encoding type class) are limited by the generated code, I'm willing to go down the path for AppBoxReference.

[algochoi]

Nice, the implementation and the tests LGTM.

Wondering what your thoughts are on naming BoxQueryEncoding something more general, like GoalEncoding, since this is a goal app-arg convention. I think this would make the class name more future proof if we want to use it in other APIs and provide additional convenience methods for it (e.g. encode to different prefixes like str and `int).

[michaeldiamant]

Wondering what your thoughts are on naming BoxQueryEncoding something more general, like GoalEncoding

My 2c is to keep as is though if you'd like to discuss further, I'll happily have a verbal.

Rationale:

• If the encoding scheme is more broadly used, it may no longer be called/considered the goal encoding.
• In the future, there may exist differences in encoding implementation for Boxes vs other concepts applying the encoding scheme. In my experience, once generalized, it's challenging to break apart concerns. And it's been easier to start with specificity and generalize later.
• Naming aside, I agree underlying logic can be shared if/when extend usage.

[algochoi]

Sounds good, LGTM 📦

[algoidurovic]

I'm slightly concerned about users not being aware of this method when calling the API but I don't see a perfect solution to it. Documentation and the locality of having the method available in the class like you're doing here is probably sufficient. Maybe more of an issue in go because of how easy it easy to convert between strings and bytes, making it pretty tempting for the user to call the endpoint with string(box.name). This PR LGTM though, just putting some thoughts out there in case you find them relevant.

[michaeldiamant]

I'm slightly concerned about users not being aware of this method

@algoidurovic Planning to merge though I share the concern. I don't see a better alternative with today's tooling. I think any alternative (e.g. defining specific types) that addresses the concern conflicts with the code generation strategy.