From b6943263d2c798c2a9925987d93c189037cc80a2 Mon Sep 17 00:00:00 2001 From: viquezclaudio <31874948+viquezclaudio@users.noreply.github.com> Date: Wed, 29 Aug 2018 08:59:18 -0600 Subject: [PATCH] Added "_" sufix to internal variables (#1171) --- CODE_STYLE.md | 18 ++++++ .../distribution/RefundableCrowdsale.sol | 14 ++--- .../SupportsInterfaceWithLookup.sol | 6 +- contracts/mocks/ERC721ReceiverMock.sol | 12 ++-- contracts/payment/Escrow.sol | 10 ++-- contracts/token/ERC20/StandardToken.sol | 55 ++++++++++--------- 6 files changed, 67 insertions(+), 48 deletions(-) diff --git a/CODE_STYLE.md b/CODE_STYLE.md index 326954820..593f35cb2 100644 --- a/CODE_STYLE.md +++ b/CODE_STYLE.md @@ -24,6 +24,24 @@ Any exception or additions specific to our project are documented below. } ``` +* Internal and private state variables should have an underscore suffix. + + ``` + contract TestContract { + uint256 internal internalVar_; + uint256 private privateVar_; + } + ``` + + Variables declared in a function should not follow this rule. + + ``` + function test() { + uint256 functionVar; + ... + } + ``` + * Events should be emitted immediately after the state change that they represent, and consequently they should be named in past tense. diff --git a/contracts/crowdsale/distribution/RefundableCrowdsale.sol b/contracts/crowdsale/distribution/RefundableCrowdsale.sol index 0342173dc..83f0ae542 100644 --- a/contracts/crowdsale/distribution/RefundableCrowdsale.sol +++ b/contracts/crowdsale/distribution/RefundableCrowdsale.sol @@ -18,7 +18,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale { uint256 public goal; // refund escrow used to hold funds while crowdsale is running - RefundEscrow private escrow; + RefundEscrow private escrow_; /** * @dev Constructor, creates RefundEscrow. @@ -26,7 +26,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale { */ constructor(uint256 _goal) public { require(_goal > 0); - escrow = new RefundEscrow(wallet); + escrow_ = new RefundEscrow(wallet); goal = _goal; } @@ -37,7 +37,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale { require(isFinalized); require(!goalReached()); - escrow.withdraw(msg.sender); + escrow_.withdraw(msg.sender); } /** @@ -53,10 +53,10 @@ contract RefundableCrowdsale is FinalizableCrowdsale { */ function finalization() internal { if (goalReached()) { - escrow.close(); - escrow.beneficiaryWithdraw(); + escrow_.close(); + escrow_.beneficiaryWithdraw(); } else { - escrow.enableRefunds(); + escrow_.enableRefunds(); } super.finalization(); @@ -66,7 +66,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale { * @dev Overrides Crowdsale fund forwarding, sending funds to escrow. */ function _forwardFunds() internal { - escrow.deposit.value(msg.value)(msg.sender); + escrow_.deposit.value(msg.value)(msg.sender); } } diff --git a/contracts/introspection/SupportsInterfaceWithLookup.sol b/contracts/introspection/SupportsInterfaceWithLookup.sol index 6e6d2d5dd..20a00088c 100644 --- a/contracts/introspection/SupportsInterfaceWithLookup.sol +++ b/contracts/introspection/SupportsInterfaceWithLookup.sol @@ -19,7 +19,7 @@ contract SupportsInterfaceWithLookup is ERC165 { /** * @dev a mapping of interface id to whether or not it's supported */ - mapping(bytes4 => bool) internal supportedInterfaces; + mapping(bytes4 => bool) internal supportedInterfaces_; /** * @dev A contract implementing SupportsInterfaceWithLookup @@ -39,7 +39,7 @@ contract SupportsInterfaceWithLookup is ERC165 { view returns (bool) { - return supportedInterfaces[_interfaceId]; + return supportedInterfaces_[_interfaceId]; } /** @@ -49,6 +49,6 @@ contract SupportsInterfaceWithLookup is ERC165 { internal { require(_interfaceId != 0xffffffff); - supportedInterfaces[_interfaceId] = true; + supportedInterfaces_[_interfaceId] = true; } } diff --git a/contracts/mocks/ERC721ReceiverMock.sol b/contracts/mocks/ERC721ReceiverMock.sol index 6fd00b6d4..82b8a2610 100644 --- a/contracts/mocks/ERC721ReceiverMock.sol +++ b/contracts/mocks/ERC721ReceiverMock.sol @@ -4,8 +4,8 @@ import "../token/ERC721/ERC721Receiver.sol"; contract ERC721ReceiverMock is ERC721Receiver { - bytes4 retval; - bool reverts; + bytes4 retval_; + bool reverts_; event Received( address _operator, @@ -16,8 +16,8 @@ contract ERC721ReceiverMock is ERC721Receiver { ); constructor(bytes4 _retval, bool _reverts) public { - retval = _retval; - reverts = _reverts; + retval_ = _retval; + reverts_ = _reverts; } function onERC721Received( @@ -29,7 +29,7 @@ contract ERC721ReceiverMock is ERC721Receiver { public returns(bytes4) { - require(!reverts); + require(!reverts_); emit Received( _operator, _from, @@ -37,6 +37,6 @@ contract ERC721ReceiverMock is ERC721Receiver { _data, gasleft() // msg.gas was deprecated in solidityv0.4.21 ); - return retval; + return retval_; } } diff --git a/contracts/payment/Escrow.sol b/contracts/payment/Escrow.sol index dfa762ebb..dc3fbfef5 100644 --- a/contracts/payment/Escrow.sol +++ b/contracts/payment/Escrow.sol @@ -17,10 +17,10 @@ contract Escrow is Ownable { event Deposited(address indexed payee, uint256 weiAmount); event Withdrawn(address indexed payee, uint256 weiAmount); - mapping(address => uint256) private deposits; + mapping(address => uint256) private deposits_; function depositsOf(address _payee) public view returns (uint256) { - return deposits[_payee]; + return deposits_[_payee]; } /** @@ -29,7 +29,7 @@ contract Escrow is Ownable { */ function deposit(address _payee) public onlyOwner payable { uint256 amount = msg.value; - deposits[_payee] = deposits[_payee].add(amount); + deposits_[_payee] = deposits_[_payee].add(amount); emit Deposited(_payee, amount); } @@ -39,10 +39,10 @@ contract Escrow is Ownable { * @param _payee The address whose funds will be withdrawn and transferred to. */ function withdraw(address _payee) public onlyOwner { - uint256 payment = deposits[_payee]; + uint256 payment = deposits_[_payee]; assert(address(this).balance >= payment); - deposits[_payee] = 0; + deposits_[_payee] = 0; _payee.transfer(payment); diff --git a/contracts/token/ERC20/StandardToken.sol b/contracts/token/ERC20/StandardToken.sol index 4eada707a..bf902a8c0 100644 --- a/contracts/token/ERC20/StandardToken.sol +++ b/contracts/token/ERC20/StandardToken.sol @@ -14,9 +14,9 @@ import "../../math/SafeMath.sol"; contract StandardToken is ERC20 { using SafeMath for uint256; - mapping (address => uint256) private balances; + mapping (address => uint256) private balances_; - mapping (address => mapping (address => uint256)) private allowed; + mapping (address => mapping (address => uint256)) private allowed_; uint256 private totalSupply_; @@ -33,7 +33,7 @@ contract StandardToken is ERC20 { * @return An uint256 representing the amount owned by the passed address. */ function balanceOf(address _owner) public view returns (uint256) { - return balances[_owner]; + return balances_[_owner]; } /** @@ -50,7 +50,7 @@ contract StandardToken is ERC20 { view returns (uint256) { - return allowed[_owner][_spender]; + return allowed_[_owner][_spender]; } /** @@ -59,11 +59,11 @@ contract StandardToken is ERC20 { * @param _value The amount to be transferred. */ function transfer(address _to, uint256 _value) public returns (bool) { - require(_value <= balances[msg.sender]); + require(_value <= balances_[msg.sender]); require(_to != address(0)); - balances[msg.sender] = balances[msg.sender].sub(_value); - balances[_to] = balances[_to].add(_value); + balances_[msg.sender] = balances_[msg.sender].sub(_value); + balances_[_to] = balances_[_to].add(_value); emit Transfer(msg.sender, _to, _value); return true; } @@ -78,7 +78,7 @@ contract StandardToken is ERC20 { * @param _value The amount of tokens to be spent. */ function approve(address _spender, uint256 _value) public returns (bool) { - allowed[msg.sender][_spender] = _value; + allowed_[msg.sender][_spender] = _value; emit Approval(msg.sender, _spender, _value); return true; } @@ -97,20 +97,20 @@ contract StandardToken is ERC20 { public returns (bool) { - require(_value <= balances[_from]); - require(_value <= allowed[_from][msg.sender]); + require(_value <= balances_[_from]); + require(_value <= allowed_[_from][msg.sender]); require(_to != address(0)); - balances[_from] = balances[_from].sub(_value); - balances[_to] = balances[_to].add(_value); - allowed[_from][msg.sender] = allowed[_from][msg.sender].sub(_value); + balances_[_from] = balances_[_from].sub(_value); + balances_[_to] = balances_[_to].add(_value); + allowed_[_from][msg.sender] = allowed_[_from][msg.sender].sub(_value); emit Transfer(_from, _to, _value); return true; } /** * @dev Increase the amount of tokens that an owner allowed to a spender. - * approve should be called when allowed[_spender] == 0. To increment + * approve should be called when allowed_[_spender] == 0. To increment * allowed value is better to use this function to avoid 2 calls (and wait until * the first transaction is mined) * From MonolithDAO Token.sol @@ -124,15 +124,15 @@ contract StandardToken is ERC20 { public returns (bool) { - allowed[msg.sender][_spender] = ( - allowed[msg.sender][_spender].add(_addedValue)); - emit Approval(msg.sender, _spender, allowed[msg.sender][_spender]); + allowed_[msg.sender][_spender] = ( + allowed_[msg.sender][_spender].add(_addedValue)); + emit Approval(msg.sender, _spender, allowed_[msg.sender][_spender]); return true; } /** * @dev Decrease the amount of tokens that an owner allowed to a spender. - * approve should be called when allowed[_spender] == 0. To decrement + * approve should be called when allowed_[_spender] == 0. To decrement * allowed value is better to use this function to avoid 2 calls (and wait until * the first transaction is mined) * From MonolithDAO Token.sol @@ -146,13 +146,13 @@ contract StandardToken is ERC20 { public returns (bool) { - uint256 oldValue = allowed[msg.sender][_spender]; + uint256 oldValue = allowed_[msg.sender][_spender]; if (_subtractedValue >= oldValue) { - allowed[msg.sender][_spender] = 0; + allowed_[msg.sender][_spender] = 0; } else { - allowed[msg.sender][_spender] = oldValue.sub(_subtractedValue); + allowed_[msg.sender][_spender] = oldValue.sub(_subtractedValue); } - emit Approval(msg.sender, _spender, allowed[msg.sender][_spender]); + emit Approval(msg.sender, _spender, allowed_[msg.sender][_spender]); return true; } @@ -166,7 +166,7 @@ contract StandardToken is ERC20 { function _mint(address _account, uint256 _amount) internal { require(_account != 0); totalSupply_ = totalSupply_.add(_amount); - balances[_account] = balances[_account].add(_amount); + balances_[_account] = balances_[_account].add(_amount); emit Transfer(address(0), _account, _amount); } @@ -178,10 +178,10 @@ contract StandardToken is ERC20 { */ function _burn(address _account, uint256 _amount) internal { require(_account != 0); - require(_amount <= balances[_account]); + require(_amount <= balances_[_account]); totalSupply_ = totalSupply_.sub(_amount); - balances[_account] = balances[_account].sub(_amount); + balances_[_account] = balances_[_account].sub(_amount); emit Transfer(_account, address(0), _amount); } @@ -193,11 +193,12 @@ contract StandardToken is ERC20 { * @param _amount The amount that will be burnt. */ function _burnFrom(address _account, uint256 _amount) internal { - require(_amount <= allowed[_account][msg.sender]); + require(_amount <= allowed_[_account][msg.sender]); // Should https://github.com/OpenZeppelin/zeppelin-solidity/issues/707 be accepted, // this function needs to emit an event with the updated approval. - allowed[_account][msg.sender] = allowed[_account][msg.sender].sub(_amount); + allowed_[_account][msg.sender] = allowed_[_account][msg.sender].sub( + _amount); _burn(_account, _amount); } }