-
Notifications
You must be signed in to change notification settings - Fork 761
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Refactor line length to rationales.md
This commit moves the bulk of our line length section over to `rationales.md`, and adds a link in the main style guide. Resolves #76
- Loading branch information
David Jarvis
committed
Feb 12, 2016
1 parent
9fcfd19
commit 5861354
Showing
2 changed files
with
87 additions
and
77 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
# Rationales | ||
|
||
This document contains what are at times lengthy rationales and justifications | ||
for the decisions made in the main [Style guide](./README.md). | ||
|
||
## Table of Contents | ||
1. [Line Length](#line-length) | ||
|
||
### Line Length | ||
|
||
Keeping code visually grouped together (as a 100-character line limit enforces) | ||
makes it easier to understand. For example, you don't have to scroll back and | ||
forth on one line to see what's going on -- you can view it all together. | ||
|
||
Here are examples from our codebase showing several techniques for | ||
breaking complex statements into multiple lines that are all < 100 | ||
characters. Notice techniques like: | ||
|
||
* liberal use of linebreaks inside unclosed `(` `{` `[` | ||
* chaining methods, ending unfinished chains with a `.` | ||
* composing long strings by putting strings next to each other, separated | ||
by a backslash-then-newline. | ||
* breaking long logical statements with linebreaks after operators like | ||
`&&` and `||` | ||
|
||
```ruby | ||
scope = Translation::Phrase.includes(:phrase_translations). | ||
joins(:phrase_screenshots). | ||
where(:phrase_screenshots => { | ||
:controller => controller_name, | ||
:action => JAROMIR_JAGR_SALUTE, | ||
}) | ||
``` | ||
|
||
```ruby | ||
translation = FactoryGirl.create( | ||
:phrase_translation, | ||
:locale => :is, | ||
:phrase => phrase, | ||
:key => 'phone_number_not_revealed_time_zone', | ||
:value => 'Símanúmerið þitt verður ekki birt. Það er aðeins hægt að hringja á '\ | ||
'milli 9:00 og 21:00 %{time_zone}.' | ||
) | ||
``` | ||
|
||
```ruby | ||
if @reservation_alteration.checkin == @reservation.start_date && | ||
@reservation_alteration.checkout == (@reservation.start_date + @reservation.nights) | ||
|
||
redirect_to_alteration @reservation_alteration | ||
end | ||
``` | ||
|
||
```erb | ||
<% if @presenter.guest_visa_russia? %> | ||
<%= icon_tile_for(I18n.t("email.reservation_confirmed_guest.visa.details_header", | ||
:default => "Visa for foreign Travelers"), | ||
:beveled_big_icon => "stamp") do %> | ||
<%= I18n.t("email.reservation_confirmed_guest.visa.russia.details_copy", | ||
:default => "Foreign guests travelling to Russia may need to obtain a visa...") %> | ||
<% end %> | ||
<% end %> | ||
``` | ||
|
||
These code snippets are very much more readable than the alternative: | ||
|
||
```ruby | ||
scope = Translation::Phrase.includes(:phrase_translations).joins(:phrase_screenshots).where(:phrase_screenshots => { :controller => controller_name, :action => JAROMIR_JAGR_SALUTE }) | ||
|
||
translation = FactoryGirl.create(:phrase_translation, :locale => :is, :phrase => phrase, :key => 'phone_number_not_revealed_time_zone', :value => 'Símanúmerið þitt verður ekki birt. Það er aðeins hægt að hringja á milli 9:00 og 21:00 %{time_zone}.') | ||
|
||
if @reservation_alteration.checkin == @reservation.start_date && @reservation_alteration.checkout == (@reservation.start_date + @reservation.nights) | ||
redirect_to_alteration @reservation_alteration | ||
end | ||
``` | ||
|
||
```erb | ||
<% if @presenter.guest_visa_russia? %> | ||
<%= icon_tile_for(I18n.t("email.reservation_confirmed_guest.visa.details_header", :default => "Visa for foreign Travelers"), :beveled_big_icon => "stamp") do %> | ||
<%= I18n.t("email.reservation_confirmed_guest.visa.russia.details_copy", :default => "Foreign guests travelling to Russia may need to obtain a visa prior to...") %> | ||
<% end %> | ||
<% end %> | ||
``` |