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

Loop issue #8

Open
ObjectPlayer opened this issue Mar 13, 2022 · 0 comments
Open

Loop issue #8

ObjectPlayer opened this issue Mar 13, 2022 · 0 comments

Comments

@ObjectPlayer
Copy link

Hey folks,
I just read your AllDay contract for learning purpose, I noticed that, there is a batchDeposit method which is getting a collection of NFTs as input parameter and deposit that NFTs in user collection. You have added loop to deposit each NFT from the collection of NFTs.

        // batchDeposit takes a Collection object as an argument
        // and deposits each contained NFT into this Collection
        //
        pub fun batchDeposit(tokens: @NonFungibleToken.Collection) {
            // Get an array of the IDs to be deposited
            let keys = tokens.getIDs()

            // Iterate through the keys in the collection and deposit each one
            for key in keys {
                self.deposit(token: <-tokens.withdraw(withdrawID: key))
            }

            // Destroy the empty Collection
            destroy tokens
        }

In my opinion there should be a limit on how many NFTs (max) will be deposit through above method, right now there is no any limit, so it will be cause an issue in case of long NFT collection, so there should be limit as pre-condition of method e.g:

        // batchDeposit takes a Collection object as an argument
        // and deposits each contained NFT into this Collection
        //
        pub fun batchDeposit(tokens: @NonFungibleToken.Collection) {
            pre{
            tokens.getIDs().length <= 10 : "maximum 10 NFTs will be deposite through batchDeposit method"
            }
            // Get an array of the IDs to be deposited
            let keys = tokens.getIDs()

            // Iterate through the keys in the collection and deposit each one
            for key in keys {
                self.deposit(token: <-tokens.withdraw(withdrawID: key))
            }

            // Destroy the empty Collection
            destroy tokens
        }

Also you can add if tokens are shouldn't be nil or at least there must be 1 NFT in the collection as pre-condition

Thanks

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