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

Refactors #282

Closed
wants to merge 2 commits into from
Closed

Refactors #282

wants to merge 2 commits into from

Conversation

Vectorized
Copy link
Collaborator

@Vectorized Vectorized commented May 16, 2022

This PR is planned to be merged after #272.

contracts/ERC721A.sol:
    Variable _currentIndex made private.
        Created _nextTokenId() internal view function to return the value instead.
    Variable _burnCounter made private.
        Created _totalBurned() internal view function to return the value instead.

Making these variables private and wrapping them in internal view functions makes them read-only.

This is intended, since devs are NOT supposed to directly edit these values.

It also makes their names more consistent with the functions _totalMinted() and _startTokenId().

Also, this will make it easier to read their values in Diamond upgradeables
(users can simply call _totalBurned() instead of ERC721AUpgradable.storage()._burnCounter).

contracts/extensions/ERC721AOwnersExplicit.sol
    Error QuantityMustBeNonZero renamed to InitializeZeroQuantity for consistency with errors in IERC721A.sol (e.g. MintZeroQuantity).
    Error AllOwnershipsHaveBeenSet renamed to AllOwnershipsInitialized.
    nextOwnerToExplicitlySet renamed to _currentIndexOwnersExplicit and made private for consistency with variables in ERC721A.sol.
        Created _nextTokenIdOwnersExplicit() internal view function to return the value instead.
    Renamed _setOwnersExplicit to _initializeOwnersExplicit to keep consistency with function _initializeOwnershipAt in ERC721A.sol.

Semantically, "initialize" is a better word than "set" for their use case, since users cannot decide on the value.

@Vectorized Vectorized closed this May 17, 2022
@Vectorized Vectorized deleted the feature/privates branch May 17, 2022 03:36
@Vectorized Vectorized restored the feature/privates branch May 17, 2022 03:39
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

Successfully merging this pull request may close these issues.

1 participant