search

Cyfrin / foundry-full-course-cu

GNU General Public License v3.0
2.88k stars 671 forks source link

Unnecessary type casting in MoodNft.sol and DeployMoodNft.s.sol contracts #1038

Closed TAdev0 closed 8 months ago

TAdev0 commented 8 months ago

Lesson 11

Operating System

macOS (Apple Silicon)

Describe the bug

Hi to the foundry course community :)

I noticed some unnecessary type castings in 2 contracts.

In MoodNft.sol : https://github.com/Cyfrin/foundry-nft-f23/blob/1e0cb4ed67e79f219889c3fac98fff0e766fd64b/src/MoodNft.sol#L95

casting the result of abi.encodePacked() into a bytes variable (array of bytes) is redondant as abi.encodePacked() result is itself an array of bytes.

Same thing happens in DeployMoodNft.s.sol : https://github.com/Cyfrin/foundry-nft-f23/blob/1e0cb4ed67e79f219889c3fac98fff0e766fd64b/script/DeployMoodNft.s.sol#L41-L43

This time, abi.encodePacked() result, which is of bytes types, is converted to string, and then back to bytes.

I removed these castings and everything compiles and works well.

I can make a PR in the MoodNft repo if you confirm this issue.

Thanks a lot to @PatrickAlphaC for this AMAZING content :) !!!!

PatrickAlphaC commented 8 months ago

Badass :)

Thanks! Could you make your PR be just a comment with your changes? Just so people watching the video can follow along?

Thank you!

TAdev0 commented 8 months ago

@PatrickAlphaC just made the PR :)