-
Notifications
You must be signed in to change notification settings - Fork 16
Style Guide
Generally, try to match the existing style of the document you're editing, don't make commits just for the sake of style changes unless they're in a separate PR just for style fixes. If you're editing code for a different purpose you can fix style issues as you encounter them.
Try to keep line lengths under 80 characters if possible. Keep in mind that strings and identifiers should be easily greppable, don't wrap lines in a way that makes it difficult to search for a string later.
If a conditional/loop line is too long and ends in a function call, break into multiple lines, where the extra lines are indented at least two tabs further than the line above, and preferably lining the parameters up vertically:
while not self.players_at_position(next_bb,
include_unseated=True,
active_only=True):
new_val = adjust_bb(next_bb)
some_val = do_some_function(argument1=True,
argument2=False,
argument3=5)
The above style is great for simpler functions that only need a few arguments wrapped, but for functions with many arguments or complex dictionaries or lists in the arguments, prefer the following style:
some_val = do_some_function(
argument1=True,
argument2=True,
argument3={
'abc': 'def',
'xyz': [123, 456, 567],
},
)
(make sure to use trailing commas)
Prefer list/dictionary/generator comprehensions over for loops where possible. Break up long comprehensions like so:
return [
(player, Event.SIT_IN, {})
for player in self.accessor.players_who_can_play()
if (not player.banned
and (player.sitting_out or player.leaving_table))
]
If the values are different based on a conditional, make sure they're at the same indentation level.
return [
(player, Event.SIT_OUT, {})
if player.leaving_table else # keep the if & else on the same line
(player, Event.SIT_IN, {})
for player in self.accessor.players_who_can_play()
]
If your line exceeds 80 characters because of operators (e.g. boolean/math), each expression should be on a new line, starting with the operator. Prefer wrapping expressions in parentheses over using backslashes. If you have nested expressions, each nest should indent futher:
if (lacks_funds(self.table.min_buyin - player.stack)
or (buyin_amt is not None
and buyin_amt > player.stack
and lacks_funds(buyin_amt - player.stack))):
raise ValueError('Player lacks funds to buy into this table')
However, nested functions or 4+ indents is usually a sign code should be refactored. For example, the code above is more readable like this:
minimum_buyin = self.table.min_buyin - player.stack
requested_buyin = buyin_amt - player.stack if buyin_amt else None
if (lacks_funds(minimum_buyin)
or (buyin_amt and lacks_funds(requested_buyin))):
raise ValueError('Player lacks funds to buy into this table')
Break up chained function calls with parens or backslashes, starting each line with .function(params)
and line up the dots:
rich_users = (
Players.objects
.filter(bank__isnull=False)
.select_related('player')
.prefetch_related('player__cashier')
.order_by('-balance')
)
- make sure to use
.only()
,.select_related()
, and.prefetch_related()
when getting QuerySets of models, make sure you know the difference between them - use
model.attrs('id', 'name', 'other_property')
to build a dictionary of values instead of{'id': model.id, 'name': model.name, ...}
We generally follow PEP8, except for:
D100
,
D101
,
D102
,
D103
,
D104
,
D105
,
D202
,
D203
,
D205
,
D400
,
E241
,
E266
,
E272
,
E701
,
W293
- make sure to follow ES6 standard coding practices: https://www.toptal.com/javascript/javascript-es6-cheat-sheet
- no semicolons
- use
let
andconst
, never usevar
unless you explicitly want lexical scope instead of block scope - use arrow functions for pure functions (almost all functions), and
function
only for functions with side effects - use
bind
instead of lambdas when you need to use functions with preset arguments e.g.onMouseEnter={onHover.bind(this, [id], true)}
instead ofonMouseEnter={function() {onHover([id], true)}}
- use ES6 template strings instead of string addition:
player-card-${id}
instead of'player-card-' + id
Don't put error cases at the end of your if
statements, instead check for all the error cases beforehand and throw early at the top, and use the main if
code only for success case logic.
In addition, try to use mappings and lists whenever possible instead of if
statements.
Bad:
function getUtensil(fruit) {
if (!fruit) {
throw new Error('No fruit!')
} else {
if (fruit == 'apple' || fruit == 'strawberry' || fruit == 'cherry' || fruit == 'cranberries') {
if (fruit == 'apple') return 'knife'
else if (fruit == 'strawberry') return 'fork'
else if (fruit == 'cherry') return 'fingers'
else return 'spoon'
} else {
throw new Error('Not a red fruit!')
}
}
}
Good:
function getUtensil(fruit) {
const red_fruits = ['apple', 'strawberry', 'cherry', 'cranberries']
const utensils = {apple: 'knife', strawberry: 'fork', cherry: 'fingers', default: 'spoon'}
if (!fruit)
throw new Error('No fruit!')
if (!red_fruits.includes(fruit))
throw new Error('Not a red fruit!')
// use a lookup with a fallback instead of a long if statement
return utensils[fruit] || utensils.default
}
Bad:
if(global.user&&global.user.keyboard_shortcuts){
while(thing()){...}
const some_func =({ arg1, arg2, arg3 })=>{
<Icon name='money' />
Good:
if (global.user && global.user.keyboard_shortcuts) {
while (thing()) {...}
const some_func = ({arg1, arg2, arg3}) => {
<Icon name="money"/>
- memoize any pure functions that take immutable.js objects as arguments (see
js/util/javascript.js
:memoize
) - don't put
myobj.toList().toJS()
inmapStateToProps
, save.toJS()
till the last possible moment in the render function, because react can compare immutable.js objects much faster than JS objects to tell if a re-render is necessary - try to keep code in render functions to a minimum of only things that need to be re-computed on each render, store data that only needs to be computed once outside the render function in a
const
const myArrowFunc = (arg) => // standalone funcs should always put () around the arg
arg * 42 + 1
const biggerValues = myList.map(item => // inline arrow funcs don't need () around the arg
(item + 1) * 2)
Put arrow function args on one line, and the function body on another. Don't use braces and return if only one expression is in the function body, e.g.:
Bad:
export const SomeComponent = ({text, className}) => {
return <div>
...
</div>
}
Good:
export const SomeComponent = ({text, className}) =>
<div>
...
</div>
Start the line with the operator, same as in Python.
const value = (expression
&& other_expression
|| (thing1 && thing2))
Use this style for ternary if-expressions (feel free to annoy @pirate if you don't like this style):
{condition ?
console.log('is true')
: console.log('is false')}
Keep the >
symbol on the same line when wrapping props on HTML/JSX elements.
<div id="12345"
class="some long class list"
onclick="abcdef">
<img id="abc"
src="/asdf/asdf/sf/sdf/"
onhover="asdfjlajsdkjfsdf"/>
</div>
Use pure functional React components instead of class myComponent extends React.Component {...}
whenever possible, they're much faster:
export const myComponent = ({text, className, onClick, ...props}) =>
<div className={className} {...props}>
<Button onClick={onClick}>{text}</Button>
</div>