Closed AntoineRondelet closed 6 years ago
While this PR might not be seen as fundamental, I believe that we should follow the style recommendations (This is just a suggestion though). The "code style" should be checked every time someone opens a PR. That way we'll keep a code base as clean (and understandable) as possible :)
This PR acts as a cleanup of the Mobius code.
A saw many inconsistencies that make the code hard/annoying to read. Thus I implemented some "solidity good practices" extracted from:
Overall, here are the things I saw the most:
require ( ! ok )
is an example of what happened to be done. See: https://solidity.readthedocs.io/en/develop/style-guide.html#whitespace-in-expressions for further details on good practices. The spacing was inconsistent for functions,if
,for
,require
...Log
as suggested in https://consensys.github.io/smart-contract-best-practices/recommendations/#differentiate-functions-and-events/* */
was used for single lines comments, sometimes//
. I tried to remove this inconsistency by adopting//
for single lines comments, and keeping the/**/
format for multiple line comments and comments preceding function declarations.is
. However, not all functions followed this (some but not all, especiallySignatureValid
that verifies if a signature is valid. Thus it has been renamed accordingly).Which is pretty confusing I think. I would suggest, that we follow the style recommendation in the solidity web page and only do this for long function/event declarations/calls. (Same thing for contract/library declaration)
if
. Sometimes,if
bodies that only had 1 line were not surrounded by braces, sometimes they were. I added braces every time as it makes the code easier to read.Moreover, I introduced some constants for the events value in the tests. I think this is better as it makes the code more flexible. If one needs to change the name of an event, one only needs to change the value of the constant. Finally, I would suggest that we adopt the convention
Test[FunctionName]
for the testing function. I didn't change it everywhere though.