-
Notifications
You must be signed in to change notification settings - Fork 143
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
1 parent
17951dc
commit e994817
Showing
1 changed file
with
19 additions
and
20 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e994817
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If swapping to arrays was better than an object,
(I disagree with the methods being an array since
metadata.viewMethods.getCounter
looks a look better thanmetadata.filter((x) => x.methodType === 'view' && x.name === 'getCounter')[0]
)Shouldn't you also swap the function data to an array of
[name, methodType, params, returns]
?The other is see with this is now you have no grouping of view and none-view, instead you have to filter on each time (or the consumer has to filter into two arrays, which could have been done by the contract).
e994817
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using an array allows for more generality, especially for statically typed languages. Also another reason for this is that our json serializer current doesn't handle nested objects. For function data, since the format is fixed, it's fine to have an object. Filtering into two arrays is fine since we do not expect a contract to have a lot of methods.