Openzeppelin-contracts: totalSupply should be a function, not state variable

[3sGgpQ8H]

In ERC20.sol and ERC20Basic.sol interfaces totalSupply is declared as a state variable rather than function:

uint256 public totalSupply;

This makes it impossible to use theses interfaces for token contracts that calculate totalSupply value like this one: https://github.com/BCAPtoken/BCAPToken/blob/004c49da7d4f58309c1fc0018dae8260ea03d8f2/sol/StandardToken.sol#L31

So, the declaration should be changed to:

function totalSupply () constant returns (uint256 result);

[frangio]

Hi @mikhail-vladimirov. The public modifier declares a function totalSupply with the required signature, but it's correct that the interface can't be used to implement something like BCAPToken because Solidity will not allow to define the totalSupply function.

We should change this.

[3sGgpQ8H]

@frangio Unfortunatelly, Solidity has bug-like feature that disallows to override public function with public variable like this:

contract Foo {
  function x () constant returns (uint);
}
contract Bar is Foo {
  uint public x; // We want to override Foo.x() with auto-generated public getter for x here
}

I hope this will be fixed one day.

[Qqwy]

So if I understand correctly:

• Solidity does not allow having a function and a property at the same time.
• There is one exception for this, which is when you make the property 'public', in which case they will have the same name, but make it impossible for child-classes to alter the way data is read from the property.
• For the properties, a lot of standardized naming, like totalSupply is used. Splitting this into a private property and public accessor (getter/setter) functions will hamper the readability of the contracts.

It seems like there is no clear path forward then. Personally I think OpenZeppelin would be the most flexible with going private properties + public accessor methods, but this would make the code slightly harder on the eyes, probably. This would be valid for the D0 and D1 goals, but would conflict with D2.

[maraoz]

Closed, this is a design decision we took to make the code simpler. I understand the drawbacks, but we'll keep the current implementation for now.

[spalladino]

I'm reopening this issue for further discussion. We have stumbled upon contracts that needed to override the totalSupply getter (such as for delegating the call to another contract under certain circumstances), and had no option but to modify the base token contracts, which is far from what we want to encourage.

@maraoz let me know if we can discuss this further.