Suggestion to rename all dynamic calculation functions

[PaulRBerg]

The following functions are mission-critical:

• getReturnableAmount
• getStreamedAmount
• getWithdrawableAmount

And they are in a different category of importance compared to the static getters (e.g. getStartTime).

This fact has become clearer once I got to implement stream statuses in #269, since I had to add special checks for the status of the stream (based on @andreivladbrg's feedback, thanks again) just in these functions but not in the other getters. In addition, I have also thought about this when I started working on invariant tests (#244) - there are a few protocol invariants that depend upon the functions above.

It is thus that I find it a little bit uncomfortable to have these functions start with the same prefix as the static getters. There's no hint at the dynamic nature of these functions.

What would you say about using one of the following prefixes instead?

• compute
• calculate

[razgraf]

The client repository suffers from a similar problem within the @sablier/models definitions. We're currently using get for the proper getter syntax on classic attributes, while anything that computes data is prefixed with find. E.g. findStatus, findStreamedAmount (these for example can be computed against a timestamp).

We have get/set as reserved keywords, find for computing and do (e.g. doUpdate, doCreateStream) for actions or heavy data setters.

Not saying it's the best name, but it's short and it does the job for now.

[PaulRBerg]

Replies

anything that computes data is prefixed with find

I am not familiar with the problem situation in the client app, but I think that find is not the optimal choice here. What about retrieve? Or fetch`?

find for computing

If find is meant for computing, then it should not be used for the static getters (getStartTime etc.).

and do for actions or ...

I am agnostic about this.

Decision Making

I gather you are also agnostic about whether we refactor the dynamic functions to use one of the prefixes suggested above, @razgraf?

I am sort of leaning towards calculate, because of this:

[razgraf]

• "Retrieve" and "fetch" are specific to data consumed from other services e.g. backends and external APIs. Using them for something else would make it very confusing.
• If you're using word-frequency as a metric, you should restrict it to sciences as a topic. "Compute" is much harder to use in a day-to-day dialogue as opposed to "Calculate". But in the context of computer science, it might not suffer from the same issue.

[PaulRBerg]

Using them for something else would make it very confusing.

A bit of a congestion in variable names there!

But in the context of computer science

Well, that's the thing, I'm not sure how much backend-y and computer science-y smart contract functions are.

They are the source of truth for the protocol, so they will invariably be used by most of our integration partners, wallets, etc. There are even automatic dapp generators that can generate a simple UI starting from the contract ABI.

That is why I would go with "calculate" - it just feels easier.

[PaulRBerg]

Yet another idea is to use the "balance of" pattern from ERC-20 and ERC-721, and do something like this:

• returnableOf
• returnableAmountOf
• streamedOf
• streamedAmountOf
• withdrawableOf
• withdrawableAmountOf

And/ or couple this with the idea I left in #227:

• senderBalanceOf or senderAmountOf (instead of returnableOf)
• streamedBalanceOf (instead of streamedOf)
• recipientBalanceOf or recipientAmountOf (instead of withdrawableOf)

[andreivladbrg]

Agree that we should rename those functions.

use the "balance of" pattern from ERC-20 and ERC-721

Interesting idea.

For me, it is either calculate or withdrawableAmountOf

[PaulRBerg]

Thanks, Andrei.

Also waiting for @razgraf's feedback (sorry for the spam lately!).

[razgraf]

These ERC-like names look good to me 👍

[PaulRBerg]

I ended up going with streamedAmountOf, returnableAmountOf, and withdrawableAmountOf.

• Rename the dynamic amount calculation functions #295
• refactor: rename dynamic amount calculation functions #296