-
Notifications
You must be signed in to change notification settings - Fork 177
ignore arrays with fewer than 2 elements #105
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,9 +80,10 @@ you are able to run it on any array you wish, in a much more direct manner. | |
|
|
||
| {% highlight coffeescript %} | ||
| do -> Array::shuffle ?= -> | ||
| for i in [@length-1..1] | ||
| j = Math.floor Math.random() * (i + 1) | ||
| [@[i], @[j]] = [@[j], @[i]] | ||
| if @length > 1 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My personal preference is to avoid placing over 50% of a function in a conditional. Instead I like to handle that logic with a short circuit not check: |
||
| for i in [@length-1..1] | ||
| j = Math.floor Math.random() * (i + 1) | ||
| [@[i], @[j]] = [@[j], @[i]] | ||
| @ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since in CoffeeScript I feel it is more obvious what is happening. Too easy to read some code and miss the one character dangling at the bottom of indented code which really changes the return value quite drastically. |
||
|
|
||
| [1..9].shuffle() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the
domakes sense.