Expose ERC721 getters internally

[nventuro]

Spawned from #1970

Currently, name, symbol, tokenURI and (soon to be added) baseURI are external. The reason for this is not open up the API unnecessarily, but in this case little harm can come from providing these methods, and having public getters would allow users (and ourselves) to implement interesting mechanisms, such as auto-generated tokenURIs.

We should change all of these getters to public.

[balajipachai]

@nventuro
Existing:

/**
* @dev Gets the token name.
* @return string representing the token name
*/
function name() external view returns (string memory) {
        return _name;
}

Proposed change:

/**
* @dev Gets the token name.
* @return string representing the token name
*/
function name() public view returns (string memory) {
        return _name;
}

Is this the correct understanding?

Why this confusion?
PR Header: Expose ERC721 getters internally
Your comment: We should change all of these getters to public.

[frangio]

That's because they are currently already exposed externally, but they should also be available internaly. public is what achieves this combination.

[nventuro]

@balajipachai correct! I meant external doesn't make the getters available internally, and we should do it (via public).

[balajipachai]

@nventuro and @frangio
Example of calling an external function from within a contract:

pragma solidity >=0.4.22 <0.6.0;

contract Test {
    uint256 public data;
    
    constructor(uint256 _data) public {
        data = _data;
    }
    
    function getData() external view returns (uint256) {
        return data;
    }
    
    function getDataFromFunctionDefinedExternal() public view returns (uint256) {
        return this.getData();
    }
}

Can't we just use this.externalFuncName wherever required within the contract instead of making them public?

[nventuro]

The this.function() syntax emits a CALL opcode, which is a much more expensive operation than an internal jump, and making the functions public doesn't create any security concerns, nor should it change the code generated when the internal getters are not used.

[balajipachai]

@nventuro Ok cool, out of curiosity how do you guys come to know that there will be CALL, DELEGATECALL etc OPCODE is involved? Could you please suggest some links wherein I could get information about this?

And yes will start changing the functions to public from external.

Thanks.

[Godtide]

ERC721 is a library, mostly inherited by other contract. The modifier external view tells the EVM that the function reads from the Blockchain and it getters available outside the contract.

"The idea is reducing gas cost!!!!!! and making the getters available outside the contract (inheritable)" not otherwise

https://ethereum.stackexchange.com/questions/19380/external-vs-public-best-practices/55426#55426

[Amxx]

name,symbol,tokenURI,baseURI are public