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

Name changes and read-only wrappers for consistency #284

Merged
merged 1 commit into from
May 17, 2022

Conversation

Vectorized
Copy link
Collaborator

@Vectorized Vectorized commented May 17, 2022

Some changes to make the naming scheme consistent,
and make certain variables private with internal view getters for read-only access.

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.

These variables are not intended for direct writing by deriving contracts.

The read-only functions are more consistent in naming with existing functions like _totalMinted() and _startTokenId().

This also simplifies accessing the values with Diamond upgradeables
(users can simply call _totalBurned() instead of ERC721AUpgradable.storage()._burnCounter).

contracts/extensions/ERC721AOwnersExplicit.sol

  • Error QuantityMustBeNonZero renamed to InitializeZeroQuantity.

    More consistent naming with other custom errors, such as MintZeroQuantity.

  • Error AllOwnershipsHaveBeenSet renamed to AllOwnershipsInitialized.

  • Variable nextOwnerToExplicitlySet renamed to _currentIndexOwnersExplicit and made private.

    Created _nextTokenIdOwnersExplicit() internal view function to return the value instead.

  • Function _setOwnersExplicit renamed to _initializeOwnersExplicit.

    More consistent naming with function _initializeOwnershipAt.

Similarly, the variable is not intended for direct writing by deriving contracts.

Semantically, "initialize" is more appropriate than "set" for their intended use cases,
as users cannot decide on the values to be written.

@Vectorized Vectorized requested a review from cygaar May 17, 2022 04:04
@Vectorized Vectorized merged commit 8f5fe4b into chiru-labs:main May 17, 2022
@Vectorized Vectorized deleted the refactor branch June 7, 2022 21: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.

None yet

2 participants