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

perf: simplify function removeStorage #233

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gzliudan
Copy link
Contributor

@gzliudan gzliudan commented Mar 26, 2022

Below is current version of function removeStorage:

    function removeStorage(string[] storage A, string memory a)
        internal
    {
        (uint256 index, bool isIn) = indexOf(A, a);
        if (!isIn) {
            revert("String not in array.");
        } else {
            uint256 lastIndex = A.length - 1; // If the array would be empty, the previous line would throw, so no underflow here
            if (index != lastIndex) { A[index] = A[lastIndex]; }
            A.pop();
        }
    }

When isIn is ture, then value a is in array A, so array A is not empty, A.length > 0, uint256 lastIndex = A.length - 1; will not underflow. And if not check (index != lastIndex) will save gas in most cases.

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