Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style Guide Corrections ;) #2

Open
jashkenas opened this issue Dec 19, 2011 · 6 comments
Open

Style Guide Corrections ;) #2

jashkenas opened this issue Dec 19, 2011 · 6 comments

Comments

@jashkenas
Copy link

Instead of doing this in a fork -- I'd rather put these in a ticket to make conversation easier...

Do not use more than one space around these operators:

# Yes
x = 1
y = 1
fooBar = 3

# No
x      = 1
y      = 1
fooBar = 3

... I think this particular point is still fairly undecided. There are a lot of sequences of CoffeeScript where aligning your left and right values to look more like a table is pleasing. I know a style guide has to pick one way or the other, but I think it's still an open question.


test: (param=null) -> # Yes
test: (param = null) -> # No

Nope -- quite the opposite. If a = b should always be written with spaces around the =, then the same holds true in default arguments, Python notwithstanding.


Ideally, improve the code to obviate the need for the
comment, and delete the comment entirely.

Actually, CoffeeScript tries to encourage pervasive commenting ... not so much to answer the "what" or the "how" of any individual line of code, but to answer the "why".

If a comment is short, the period at the end can be omitted.

Nope -- comments should be written as full sentences, with proper punctuation.


The bit that you describe as "block comments" aren't CoffeeScript block comments, which look like this:

###
comment
###

... but, the ones that you describe are always preferred instead of block comments -- which should only be used if you intend to pass along the comment to the underlying JS, like a license, or warning.


The use of inline comments should be limited, because their existence is typically a sign of a code smell.

Nope -- poor comments are poor, and good comments are good. There's no problem with writing good inline comments to explain an un-obvious function or tricky algorithm.


For constants, use all uppercase with underscores

I don't think that this one is settled quite yet. In JavaScript, of course, until const lands, there's no such thing as a constant. For that reason, I think that simply using local variable style for constants may actually make more sense. I'm not sure which should be prescribed.


When calling functions, omit the parentheses on the final method call in a chain.

I think that parentheses inclusion or omission doesn't have a hard and fast rule. For example, your second example doesn't benefit:

foo(4).bar(8) # ... is probably more readable than:
foo(4).bar 8

Usually, it should be obvious (or at least obvious-ish) when leaving parentheses (and braces) off makes things clearer, and when it doesn't:

brush.ellipse x: 10, y: 20

obj.value(10, 20) / obj.value(20, 10)

print inspect value

new Tag(new Value(a, b), new Arg(c))

The correct way to apply the function grouping style when chaining is to use it for the initial call only.

I personally don't use it, or recommend it, but I would think that folks that do prefer it would take issue with this rule.


Finally, you may want to add a note to always avoid use of standalone @, all by itself. A simple this is preferred. (A common question.)


Thanks for this style guide -- it's really great stuff.

@mjrusso
Copy link
Member

mjrusso commented Jan 6, 2012

Jeremy,

This is excellent feedback and is much appreciated. Thank you!


If a = b should always be written with spaces around the =, then the same holds true in default arguments, Python notwithstanding.

Changed here: 233ea6a


All of the issues with this guide's treatment of comments have been moved to #6.


you may want to add a note to always avoid use of standalone @, all by itself. A simple this is preferred. (A common question.)

Note added here: 05ad9c6


In JavaScript, of course, until const lands, there's no such thing as a constant. For that reason, I think that simply using local variable style for constants may actually make more sense. I'm not sure which should be prescribed.

Moving this to #3 for further discussion.


I think that parentheses inclusion or omission doesn't have a hard and fast rule.

I agree that the focus should be on readability, not following a rule for the rule's sake.

An attempt at communicating this (using your examples): d3ccb1f

I personally don't use it, or recommend it, but I would think that folks that do prefer it would take issue with this rule.

Agreed:


I think this particular point is still fairly undecided. There are a lot of sequences of CoffeeScript where aligning your left and right values to look more like a table is pleasing. I know a style guide has to pick one way or the other, but I think it's still an open question.

This alignment can be visually pleasing in certain scenarios, but the drawbacks also need to be considered (such as the increased maintenance burden).

I've included a few examples below, directly from the docco source. (My intention isn't to pick on docco specifically, but rather to demonstrate how hard it is to consistently apply this alignment rule.)

This looks nice:

fs       = require 'fs'
path     = require 'path'
showdown = require('./../vendor/showdown').Showdown

Including the next line:

fs       = require 'fs'
path     = require 'path'
showdown = require('./../vendor/showdown').Showdown
{spawn, exec} = require 'child_process'

Is the inconsistency deliberate?

Later on:

# Create the template that we will use to generate the Docco HTML page.
docco_template  = template fs.readFileSync(__dirname + '/../resources/docco.jst').toString()

# The CSS styles we'd like to apply to the documentation.
docco_styles    = fs.readFileSync(__dirname + '/../resources/docco.css').toString()

This is consistent alignment, but I would argue that it looks somewhat awkward with the comments interspersed. (The statements look better when viewing the output that docco generates when run against its own source. Which reading mode is more important to optimize for?)

A few lines down and there is another example of an inconsistency:

# The end of each Pygments highlight block.
highlight_end   = '</pre></div>'

# Run the script.
# For each source file passed in as an argument, generate the documentation.
sources = process.ARGV.sort()

Overall it seems unrealistic to mandate this sort of alignment across-the-board.

A reasonable compromise could read something like: "In general, do not align left and right values in assignment statements, unless an uninterrupted sequence of assignment statements follow each other."

With this rule, the following code (also taken exactly from docco) is compliant:

parse = (source, code) ->
  lines    = code.split '\n'
  sections = []
  language = get_language source
  has_code = docs_text = code_text = ''

  save = (docs, code) ->
    sections.push docs_text: docs, code_text: code

And an earlier example could validly look like this:

fs            = require 'fs'
path          = require 'path'
showdown      = require('./../vendor/showdown').Showdown
{spawn, exec} = require 'child_process'

Or like this:

fs       = require 'fs'
path     = require 'path'
showdown = require('./../vendor/showdown').Showdown

{spawn, exec} = require 'child_process'

I'm not sure that I'm sold on this rule, but it's worth mulling over further.

Thanks,
Michael

@jashkenas
Copy link
Author

In general, do not align left and right values in assignment
statements, unless an uninterrupted sequence of assignment
statements follow each other.

... I dig it. That's a nice rule. And even then, it's an opt-in thing -- you don't have to do it.

@micahlmartin
Copy link

I find that omitting the () from a function definition with no params makes it harder to read.

I find this

foo = () - >

In the first case the () give clear separation from the = whereas the second examples muddles together the = and the -> making it harder to read imho.
to much more readable than this

food = ->

@andreineculau
Copy link

Why is the standalone @ not ok?

Overall, I like consistency unless there's a functional disadvantage. And in this case it's also usability: search only for @, not both @ and this

@paulmillr
Copy link

@andreineculau it looks shitty

@vlcinsky
Copy link

vlcinsky commented Aug 9, 2013

Do you know, why @ is used for "this."?
@paulmillr revealed the truth, that it looks shitty. Not only the shape, but also the spelling.

One can make "this" from characters of "shit".

shitChars = 'shit'.split('')
thisChars = [shitChars.pop(), shitChars.splice(1,1)[0], shitChars.pop(), shitChars.pop()]
console.assert thisChars.join('') is 'this', 'word seems to be improved'
console.assert shitChars.length is 0, 'no pieces of old "shit" are left'

Now seriously - I think, if we start using "@" (and we should) anywhere in the code, we shall keep it consistent for standalone use too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants