chore(Docs): Replaced instances if - throw to require() where applicable.#2207
chore(Docs): Replaced instances if - throw to require() where applicable.#2207
Conversation
| :: | ||
|
|
||
| pragma solidity ^0.4.0; | ||
| pragma solidity ^0.4.11; |
There was a problem hiding this comment.
Shouldn't 0.4.10 be enough here for transfer?
There was a problem hiding this comment.
Let's keep it there, people should upgrade.
docs/frequently-asked-questions.rst
Outdated
|
|
||
| if (balanceOf[_to] + _value < balanceOf[_to]) | ||
| throw; | ||
| require(balanceOf[_to] + _value >= balanceOf[_to]); |
There was a problem hiding this comment.
I think we should have parentheses here.
There was a problem hiding this comment.
These are the FAQ, hey are horribly outdated.
docs/security-considerations.rst
Outdated
| write your contract using a pattern where the recipient can withdraw Ether instead. | ||
| 3. Sending Ether can also fail because the execution of the recipient contract | ||
| requires more than the allotted amount of gas (explicitly by using ``throw`` or | ||
| requires more than the allotted amount of gas (explicitly by using ``revert`` or |
There was a problem hiding this comment.
Perhaps extend that to:
"explicitly by using revert, require or assert"
docs/solidity-by-example.rst
Outdated
| // called incorrectly. But watch out, this | ||
| // will currently also consume all provided gas | ||
| // (this is planned to change in the future). | ||
| require(msg.sender == chairperson || !voters[voter].voted); |
There was a problem hiding this comment.
Please add parentheses to the left.
docs/solidity-by-example.rst
Outdated
|
|
||
| // Revert the call if the bidding | ||
| // period is over. | ||
| require(now < auctionStart + biddingTime); |
There was a problem hiding this comment.
Parentheses wouldn't hurt here. Also <=.
docs/solidity-by-example.rst
Outdated
|
|
||
| // If the bid is not higher, send the | ||
| // money back. | ||
| require(msg.value <= highestBid); |
docs/solidity-by-example.rst
Outdated
| throw; // auction did not yet end | ||
| if (ended) | ||
| throw; // this function has already been called | ||
| require(now >= auctionStart + biddingTime); // auction did not yet end |
There was a problem hiding this comment.
I really don't like to add parentheses around all operations. This is not javascript, it would make no sense to have (now >= auctionStart) + biddingTime...
There was a problem hiding this comment.
I will add them here, though, because it is one of the first examples.
There was a problem hiding this comment.
I know the type check would catch it if the precedence resolution would be broken, but I don't think it is bad practice having parentheses.
docs/solidity-by-example.rst
Outdated
| throw; | ||
| } | ||
| require( | ||
| _values.length == length && |
There was a problem hiding this comment.
I don't think we should do that...
I will change it into three separate requires for readability.
docs/solidity-by-example.rst
Outdated
| seller = msg.sender; | ||
| value = msg.value / 2; | ||
| if (2 * value != msg.value) throw; | ||
| require(2 * value == msg.value); |
There was a problem hiding this comment.
Parentheses could help readability here too
There was a problem hiding this comment.
I actually don't think so. it is a simple mathematical equation, nobody would add parentheses there.
docs/solidity-by-example.rst
Outdated
| function confirmPurchase() | ||
| inState(State.Created) | ||
| require(msg.value == 2 * value) | ||
| condition(msg.value == 2 * value) |
There was a problem hiding this comment.
Parentheses for readability :)
| // block the refund. | ||
| if (!buyer.send(value) || !seller.send(this.balance)) | ||
| throw; | ||
| // block the refund - the withdraw pattern should be used. |
There was a problem hiding this comment.
Maybe make the withdraw patter comment more dominant, with putting it on a new line: Note: the with...
2 participants
Fixes #2134 fixes #2118