Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review code reuse pattern #4

Closed
fulldecent opened this issue Jul 13, 2019 · 2 comments
Closed

Review code reuse pattern #4

fulldecent opened this issue Jul 13, 2019 · 2 comments

Comments

@fulldecent
Copy link
Owner

fulldecent commented Jul 13, 2019

Motivation

Most new contributors to Aion will start directly by copy-pasting the code here or from another real world example reference project. To help these people, this project should be of the highest quality possible and also make references to all relevant areas where somebody might want to learn more.

Issue

Previously this was written as a combined interface + reference implementation + mock + test mock, that's bad. Now it is spread out to multiple files.

Instead, the interface (AIP040) should be separate from the reference implementation (nf-token.java) and the mock (nf-token-test-mock.java).

This is handled beautifully in 0xcert's reference implementation of non-fungible tokens on EVM:

Reference:
https://github.com/0xcert/ethereum-erc721/tree/master/src/contracts/tokens
https://github.com/0xcert/ethereum-erc721/tree/master/src/tests/mocks

We should decide how to implement this code modularity in a Java-friendly way. Perhaps an interface with default function implementations will be preferred.

Other notes:

  • People love mixins for smart contracts, like pulling in isOwnable and erc721 and passable. We should find how to support this extremely common use case. And interfaces may support this.
@fulldecent
Copy link
Owner Author

fulldecent commented Aug 4, 2019

Call site code and storage were separated out in b2ae784.

Current code is organized as:

  • Can go upstream eventually
    • src/main/java/org/aion/AVMBlockchainWrapper.java
  • Standard
    • src/main/java/org/aion/AIP040Events.java
    • src/main/java/org/aion/AIP040Encoder.java
  • Reference implementation
    • src/main/java/org/aion/NFTokenStorage.java
    • src/main/java/org/aion/NFToken.java
    • src/main/java/org/aion/NFTokenEncoder.java
  • Mock implementation (adding features that are non-standard but which people surely want)
    • src/main/java/org/aion/NFTokenMockEncoder.java
    • src/main/java/org/aion/NFTokenMock.java
  • Entrypoint
    • src/main/java/org/aion/Main.java
    • src/main/java/org/aion/MainEncoder.java

Work plan

  • Review if this pattern of code organization does support the identified code reuse use cases.
  • Decide if Main.java is best practice. Currently a rule "put all your @Callables in one file" is enforced. Is this good or should the @Callables be separated into the different files?
  • Review if storage implementation here is best practice and that AVMBlockchainWrapper.java nearly represents what might be implement upstream at some point.

Requesting help: @jeff-aion @jennijuju

@fulldecent
Copy link
Owner Author

Reviewed with team and agreed that this is the approach to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant