summary refs log tree commit diff stats
path: root/doc
diff options
context:
space:
mode:
authorTimothee Cour <timothee.cour2@gmail.com>2018-10-18 11:20:26 -0700
committerAndreas Rumpf <rumpf_a@web.de>2018-10-18 20:20:26 +0200
commit15dbd973dec848f1c429fe7043e53254a501812b (patch)
treeae6ed5f71631210d22ae61ddf5d5bc2dfaaa1aa4 /doc
parent9f18a4f4486d8eb7efd28f51c32d7b2f8c3018fa (diff)
downloadNim-15dbd973dec848f1c429fe7043e53254a501812b.tar.gz
[doc] start of best practices section in contributing.rst (#9415)
Diffstat (limited to 'doc')
-rw-r--r--doc/contributing.rst80
1 files changed, 80 insertions, 0 deletions
diff --git a/doc/contributing.rst b/doc/contributing.rst
index 5cb79d313..fedabab11 100644
--- a/doc/contributing.rst
+++ b/doc/contributing.rst
@@ -248,6 +248,86 @@ or
 
 the first is preferred.
 
+Best practices
+=============
+
+Note: these are general guidelines, not hard rules; there are always exceptions.
+Code reviews can just point to a specific section here to save time and
+propagate best practices.
+
+.. _noimplicitbool:
+Take advantage of no implicit bool conversion
+
+.. code-block:: nim
+
+  doAssert isValid() == true
+  doAssert isValid() # preferred
+
+.. _immediately_invoked_lambdas:
+Immediately invoked lambdas (https://en.wikipedia.org/wiki/Immediately-invoked_function_expression)
+
+.. code-block:: nim
+
+  let a = (proc (): auto = getFoo())()
+  let a = block:  # preferred
+    getFoo()
+
+.. _design_for_mcs:
+Design with method call syntax (UFCS in other languages) chaining in mind
+
+.. code-block:: nim
+
+  proc foo(cond: bool, lines: seq[string]) # bad
+  proc foo(lines: seq[string], cond: bool) # preferred
+  # can be called as: `getLines().foo(false)`
+
+.. _avoid_quit:
+Use exceptions (including assert / doAssert) instead of ``quit``
+rationale: https://forum.nim-lang.org/t/4089
+
+.. code-block:: nim
+
+  quit() # bad in almost all cases
+  doAssert() # preferred
+
+.. _tests_use_doAssert:
+Use ``doAssert`` (or ``require``, etc), not ``assert`` in all tests.
+
+.. code-block:: nim
+
+  runnableExamples: assert foo() # bad
+  runnableExamples: doAssert foo() # preferred
+
+.. _delegate_printing:
+Delegate printing to caller: return ``string`` instead of calling ``echo``
+rationale: it's more flexible (eg allows caller to call custom printing,
+including prepending location info, writing to log files, etc).
+
+.. code-block:: nim
+
+  proc foo() = echo "bar" # bad
+  proc foo(): string = "bar" # preferred (usually)
+
+.. _use_Option:
+[Ongoing debate] Consider using Option instead of return bool + var argument,
+unless stack allocation is needed (eg for efficiency).
+
+.. code-block:: nim
+
+  proc foo(a: var Bar): bool
+  proc foo(): Option[Bar]
+
+.. _use_doAssert_not_echo:
+Tests (including in testament) should always prefer assertions over ``echo``,
+except when that's not possible. It's more precise, easier for readers and
+maintaners to where expected values refer to. See for example
+https://github.com/nim-lang/Nim/pull/9335 and https://forum.nim-lang.org/t/4089
+
+.. code-block:: nim
+
+  echo foo() # adds a line in testament `discard` block.
+  doAssert foo() == [1, 2] # preferred, except when not possible to do so.
+
 The Git stuff
 =============