# ENS-Nifty Code Review This review was performed upon the commit hash [b5076d7](https://github.com/ENS-Nifty/ens-nifty-contracts/tree/b5076d7b7523ab319c04ffe835f0e8241369dbd0). ## Suggestions - No reason to copy in ```zeppelin-solidity``` and ```@ensdomains/ens``` if you use ```npm``` which you are already using. ## ENSNFT.sol This contract extends various ```Open Zeppelin``` contracts including: - ERC721Token - Ownable ### Praises - Good idea to keep ```Metadata``` handling external. ### Suggestions - Stick to identical code style when importing, use either ```'``` or ```"``` but not a combination of both. - Remove ```Transfer``` event definition. Open Zeppelin already defines this. ### tokenUri Kind of hacky, but an interesting solution nonetheless. No other way to do this which I can currently think of. ### mint This function looks fine. ### burn Also looks perfectly fine. ## Metadata.sol ```strings``` is used for all variables. ### tokenURI This function looks fine, it attaches a unique ID to the end of a url. ### uint2hexstr An efficient implementation for this can be found [here](https://github.com/ensdomains/ens/blob/master/contracts/ReverseRegistrar.sol#L101). The hashing part can be ignored.