literallymarvellous / Marvel-nft

100 nft collection
0 stars 0 forks source link

Remarks #1

Open PradhumnaPancholi opened 2 years ago

PradhumnaPancholi commented 2 years ago
  1. No implementation of IPFS hosted asset in the token.
  2. 2 Not comments (grades were not deducted for this).
  3. No way to confirm if the contract was deployed and verified on testnet

Missed opportunity to used method overloading which could have made the modifiers not necessary. This one is important because modifiers were created to do not repeat the code but the mint function's logic is repeated. It could have been avoided and it would mitigate the need to create modifiers.

literallymarvellous commented 2 years ago
  1. The tokens are hosted on ipfs, although I didn't create metadata for all the tokens, the tokenURI function returns an ipfs link
  2. Totally missed that, will add comments.
  3. Noted, this is on me, It's deployed and verified, I failed to push the commit where I added that to the readme.

Saw your example of method overloading during the last office hour, and will make changes to the code.

Thanks for the feedback