Style: Improve readability on ERC 721 parameter naming in approval functions

[farismln]

Compose aim for onchain readability, with that in mind I want to propose some change on the current ERC 721 parameter naming in approval function.

The parameter name _approved is used in both approve and setApprovalForAll but it is represent two different data types (address and boolean):

    function approve(address _approved, uint256 _tokenId) external {
...
@>      s.approved[_tokenId] = _approved;
        emit Approval(owner, _approved, _tokenId);
    }

    function setApprovalForAll(address _operator, bool _approved) external {
        if (_operator == address(0)) {
            revert ERC721InvalidOperator(_operator);
        }
@>      getStorage().isApprovedForAll[msg.sender][_operator] = _approved;
        emit ApprovalForAll(msg.sender, _operator, _approved);
    }

Recommendation:

1. _approved used for boolean parameter. (no change in current implementation of setApprovalForAll)
2. _operator or _to is used for address parameter. this is the only change. affect only the approve function

Note: this deviate from EIP-721 interface examples, but it does not necessarily became non-compliant

[CodeCryptX]

Yes, thats a good recommendation to make it easier to understand.

[mudgen]

Yes, makes sense. I'm fine with this change. I prefer _to name in approve function because that distinguishes it from _operator in setApprovalForAll

[farismln]

hi @mudgen I just noticed this _approved dual data types also happen in the event.

    /// @notice Emitted when the approved address for an NFT is changed or reaffirmed.
    event Approval(address indexed _owner, address indexed _approved, uint256 indexed _tokenId);

    /// @notice Emitted when an operator is enabled or disabled for an owner.
    event ApprovalForAll(address indexed _owner, address indexed _operator, bool _approved);

might as well change the event Approval -> _approved into _to. what do you think?

I will create the PR if you dont mind

[mudgen]

Yes, sounds good.