Added KCTWrapper and Wrapper classes for KIP* #223
Conversation
This commit is for improving SDK usability of KCT Layer in Common Architecture Improving SDK usability means providing users with a similar development experience no matter what programming language the SDK is implemented in. Each layer declared in Common Architecture should be accessed via caver. KCT Layer in Common Architecture should be accessed via `caver.kct`. In caver-js, all of static methods declared in `KIP*` classes are accessed via `caver.kct.kip*` e.g., `caver.kct.kip7.detectInterface()`, `caver.kct.kip17.deploy()`, etc… To make similar development experiences in caver-java, I added some wrapper classes. 1. Add Wrappers for KIP7, KIP17, KIP37 By adding wrapper classes for KIP7, KIP17, KIP37, we can use the static methods defined in the KIP of that type through `caver.kct.kip*`. Each wrapper classes has a Caver instance as a member variable. This allows SDK users to call static methods defined in the KIP* without passing a Caver type instance and IWallet type instance as method parameters. This member variable caver should be injected from KCTWrapper. 2. Add KCTWrapper and add this as a member variable of Caver class KCTWrapper contains wrapper classes of KIP* which have static methods. By adding this class and adding this as a member variable of Caver class, we can use static methods declared in the KIP* through `caver.kct.kip*` e.g., `caver.kct.kip7.detectInterface()`
| * }</pre> | ||
| * | ||
| * @param contractAddress | ||
| * @return |
There was a problem hiding this comment.
| * @return | |
| * @return Map<String, Boolean> |
There was a problem hiding this comment.
@sirano11
Thanks for your feedback.
I'll check whether there are other missing types at @return and if there are, I'll update missed types.
|
|
||
| /** | ||
| * Deploy a KIP-17 contract. | ||
| * It must add deployer's keyring in caver.wallet. |
There was a problem hiding this comment.
| * It must add deployer's keyring in caver.wallet. | |
| * The deployer's keyring should be existed in `caver.wallet`. |
There was a problem hiding this comment.
@jimni1222
Thanks for your feedback.
I'll update comments of KIP*Wrappers.
core/src/main/java/com/klaytn/caver/kct/kip_wrapper/KIP17Wrapper.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/klaytn/caver/kct/kip_wrapper/KIP7Wrapper.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/klaytn/caver/kct/kip_wrapper/KIP7Wrapper.java
Outdated
Show resolved
Hide resolved
| public KIP17Wrapper(Caver caver) { | ||
| this.caver = caver; | ||
| } | ||
|
|
There was a problem hiding this comment.
@aeharvlee Don't we need to add a create()?
| public KIP37Wrapper(Caver caver) { | ||
| this.caver = caver; | ||
| } | ||
|
|
There was a problem hiding this comment.
@aeharvlee Don't we need to add a create()?
| public KIP7Wrapper(Caver caver) { | ||
| this.caver = caver; | ||
| } | ||
|
|
There was a problem hiding this comment.
@aeharvlee Don't we need to add a create()?
There was a problem hiding this comment.
@sirano11
I checked the KCT Layer in Common Architecture and yeah there was create().
I think we need to add create() for KIP7, KIP17, and KIP37.
Sorry to missed it T_T.
I'll add implementation about it.
Thanks for your feedback.
There was a problem hiding this comment.
@sirano11
Hi, I added create() methods for KIP*Wrapper classes based on your feedback and KCT Layer Reference.
Here is the related commit :)
Add create methods in KIP*Wrapper
Thanks for your feedback again.
core/src/main/java/com/klaytn/caver/kct/kip_wrapper/KIP7Wrapper.java
Outdated
Show resolved
Hide resolved
KIP7, KIP17, and KIP37 have `create()` methods in Common Architecture Design. Because The Wrapper classes were added for implementing Common Architecture, they must have `create()` method in them.
3 participants
Proposed changes
This PR is for improving SDK usability of KCT Layer in Common Architecture
Improving SDK usability means providing users with a similar development experience
no matter what programming language the SDK is implemented in.
Each layer declared in Common Architecture should be accessed via
caver.KCT Layer in Common Architecture should be accessed via
caver.kct.In caver-js, all of static methods declared in
KIP*classes are accessed viacaver.kct.kip*e.g.,
caver.kct.kip7.detectInterface(),caver.kct.kip17.deploy(), etc…To make similar development experiences in caver-java, I added some wrapper classes.
1. Add Wrappers for KIP7, KIP17, KIP37
By adding wrapper classes for
KIP7,KIP17,KIP37, we can usethe static methods defined in the KIP of that type through
caver.kct.kip*.Each wrapper classes has a
Caverinstance as a member variable.This allows SDK users to call static methods defined in the
KIP*without passing a
Cavertype instance andIWallettype instance as method parameters.This member variable caver should be injected from
KCTWrapper.2. Add KCTWrapper and add this as a member variable of Caver class
KCTWrappercontains wrapper classes ofKIP*which have static methods.By adding this class and adding this as a member variable of
Caverclass,we can use static methods declared in the
KIP*throughcaver.kct.kip*e.g.,
caver.kct.kip7.detectInterface()Types of changes
Please put an x in the boxes related to your change.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
Related issues
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...