SIP for clarity coding standards
LNow opened this issue · 10 comments
I believe that as a community we should have clear coding standards and guidelines that we want to be followed by developers when they work on clarity contracts.
I drafted something like that for Syvita guild, and we could use it as a starting point.
Naming conventions
Using language reserved keywords (data types, function names and keywords) in any other context than they were design for is prohibited.
For example:
;; BAD
(define-map MyMap
{ block-height: uint }
{ len: uint}
)
;; GOOD
(define-map MyMap
{ stx-block-height: uint }
{ length: uint}
)
Constants
Constants have to be declared in all upper case with underscore separators.
For example:
(define-constant CONTRACT_OWNER tx-sender)
(define-constant ERR_UNAUTHORIZED (err "Unauthorized!"))
Local and persisted variables
Local and persisted variables have to be declared in camelCase. For example:
(define-data-var blockSize uint u128)
(let
(
(firstVar (+ 1 10))
(secondVar (pow u2 u8))
)
(ok (call-function firstVar secondVar))
)
Maps
Maps have to be declared in PascaCase. Key tuple definition and Map tuple definition should be declared in separate lines and follow tuples standard. For example:
(define-map UserRoles
{user: principal}
{
roles: uint,
isActive: bool
}
)
To save space and reduce execution costs both key and value can be declared as unnamed data types.
For example:
(define-map UserId
principal
uint
)
(define-map UserRoles
principal
{
roles: uint,
isActive: bool
}
)
Tuples
Tuples have to be declared using curlybracket notation. Each tuple key have to be defined in separate line and follow local and persisted variables standard. For example:
{
user: tx-sender,
roles: u1,
group: "A",
membersCount: 123
}
Functions
Functions have to be declared in all lower case with dash separators. Function parameters should follow local and persisted variable standards For example:
(define-read-only (is-root (userName (buff 50)))
(ok true)
)
(define-read-only (is-member (userId uint) (groupId uint))
(false)
)
Errors
Custom errors should be defined as constants unsigned integers greater than 100, to be easily distinguishable from errors raised by build in functions. Name of each error should start with ERR_
followed by clear description of error.
For example:
(define-constant ERR_UNAUTHORIZED u401)
(define-public (some-function)
(asserts! (is-eq tx-sender contract-caller)
(err ERR_UNAUTHORIZED)
)
)
Closing parenthesis
Parenthesis should be closed in the same line if line is very short or at the same level as they were opened if what is inside parenthesis is more complex. For example
;; BAD
(define-public (do-something-cool (userId uint))
(let
((firstVar (+ 1 10))
(secondVar (pow u2 u8)))
(ok (call-function firstVar secondVar))))
;; GOOD
(define-public (do-something-cool (userId uint))
(let
(
(firstVar (+ 1 10))
(secondVar (pow u2 u8))
)
(ok (call-function firstVar secondVar))
)
)
@LNow Thank you for kicking this off, excellent start.
Friedger also voiced the idea for standard error codes should that be part of this SIP or separate? To me it seems reasonable to make it part of this more general idea for a clarity coding standard.
If he was talking about error codes returned by native function, I think they need to be and are described in language reference.
Yet, I see nothing against listing them in codding standards as well.
I think this is a good idea! While the blockchain itself can't enforce a coding standard, a SIP that describes a coding standard could be used to inform the implementation of developer tooling.
I like this idea too. Thanks @LNow! I'd happily add a linter pass to Clarinet once the styles are agreed upon.
We used this standard to help make the CityCoins contracts much more readable and almost one year later it was totally worth it.
The fact you can look the code and quickly distinguish between functions, variables, maps, and constants is very useful especially as the complexity of the contract increases.
I define my TypeScript interfaces/types with PascalCase, which fits perfectly with defining the data structure within a map, e.g.
Properly laying out the parentheses is extremely helpful too. If someone is coming into Clarity with no experience with a LISP-like language, it helps with getting used to the nuances of how the code is structured, e.g. using let
:
(define-public (double-foo (foo uint))
(let
(
;; define let variables here
(bar (* foo u2))
)
;; operations / return
(ok bar)
)
)
It also helps with realizing where you can simplify things, e.g. the function above could avoid the let
statement completely:
(define-public (double-foo (foo uint))
(ok (* foo u2))
)
One more thing to note - if voted in, this format would be extremely helpful for contract documentation as well, especially the page for function references, e.g.
(let ((a 2) (b (+ 5 6 7))) (print a) (print b) (+ a b)) ;; Returns 20
(let ((a 5) (c (+ a 1)) (d (+ c 1)) (b (+ a c d))) (print a) (print b) (+ a b)) ;; Returns 23
@LNow any thoughts on the error format here? Recently we discussed doing it one of two ways:
Current:
(define-constant ERR_UNAUTHORIZED u401)
(define-public (some-function)
(asserts! (is-eq tx-sender contract-caller)
(err ERR_UNAUTHORIZED)
)
)
Updated method:
(define-constant ERR_UNAUTHORIZED (err u401))
(define-public (some-function)
(asserts! (is-eq tx-sender contract-caller)
(ERR_UNAUTHORIZED)
)
)
Most of the time I prefer updated version.
@obycode I closed this and few other issues because they were obsolete and I've lost all interest to do anything but close them.
Is this a great improvement that is beneficial to the all of Stacks?
If so, I'd really love to see it revived somehow.