Skip to content

Latest commit

History

History

Folders and files

NameName
Last commit message
Last commit date

parent directory

..

README.md

Prelude

Invoca Ruby Style Guide initially forked from https://github.com/bbatsov/ruby-style-guide.

The contents of this styleguide are to be enforced by rubocop and automated in the code review process by HoundCI

Any adjustments to this styleguide should be captured in the .rubocop.yml config.

Table of Contents

Source Code Layout

Use two spaces per indentation level. No hard tabs.

# gooddefsome_methoddo_somethingend# bad - four spacesdefsome_methoddo_somethingend

Use spaces around operators and =>, after commas, colons and semicolons, around {

and before }. (But there is no need for spaces inside the empty hash {}.)

a,b=1,2 + 3location={:city=>'Santa Barbara',:state=>'CA'}size > 10 ? 'large' : 'small'[1,2,3].each{ |e| putse}params={}

The only exception is when using the exponent operator:

# bade=M * c ** 2# goode=M * c**2

No spaces after (, [ or before ], ).

some(arg).other[1,2,3].lengthcollection=[]

Indent when as deep as case. I know that many would disagree.

with this one, but it's the style established in both the "The Ruby Programming Language" and "Programming Ruby".

casewhensong.name == 'Misty'puts'Not again!'whensong.duration > 120puts'Too long!'whenTime.now.hour > 21puts"It's too late"elsesong.playendkind=caseyearwhen1850..1889then'Blues'when1890..1909then'Ragtime'when1910..1929then'New Orleans Jazz'when1930..1939then'Swing'when1940..1950then'Bebop'else'Jazz'end

Use empty lines between defs and to break up a method into logical paragraphs.

defsome_methoddata=initialize(options)data.manipulate!data.resultenddefsome_methodresultend

Align the parameters of a method call if they span over multiple lines using normal indent.

# starting point (line is too long)defsend_mail(source)Mailer.deliver(to: 'bob@example.com',from: 'us@example.com',subject: 'Important message',body: source.text)end# good (normal indent)defsend_mail(source)Mailer.deliver(to: 'bob@example.com',from: 'us@example.com',subject: 'Important message',body: source.text)end# bad (double indent)defsend_mail(source)Mailer.deliver(to: 'bob@example.com',from: 'us@example.com',subject: 'Important message',body: source.text)end# baddefsend_mail(source)Mailer.deliver(to: 'bob@example.com',from: 'us@example.com',subject: 'Important message',body: source.text)end

Prefer leading dot notation when chaining method calls on multiple lines

# baddeftrailing_dotmethod.chain.chainend# gooddefleading_dotmethod.chain.chainend

Align multiline method call chains with an indent relative the receiver

# badcollection.transformA{ ... }.transformB{ ... }# goodcollection.transformA{ ... }.transformB{ ... }# goodwhilemyvariable.a.b# do somethingend# goodmyvariable=Thing.a.b.c

Use RDoc and its conventions for API documentation. ### Use RDoc

Use RDoc and its conventions for API documentation.

Don't put an empty line between the comment block and the def.

Avoid lines longer than 150 characters.

Avoid trailing whitespace.

Use UTF-8 as the source file encoding when you need more than plain ASCII.

Syntax

Prefer parentheses around arguments in method declaration.

Omit the parentheses when the method doesn't accept any arguments.

defsome_method
...
enddefsome_method_with_arguments(arg1,arg2)
...
end

Never use for, unless you know exactly why.

Most of the time iterators should be used instead. for is implemented in terms of each (so you're adding a level of indirection), but with a twist - for doesn't introduce a new scope (unlike each) and variables defined in its block will be visible outside it.

arr=[1,2,3]# badforeleminarrdoputselemend# goodarr.each{ |elem| putselem}

Never use then for multi-line if/unless.

# badifsome_conditionthen
...
end# goodifsome_condition
...
end

Use one expression per branch in a ternary operator.

This also means that ternary operators must not be nested. Prefer if/else constructs in these cases. Avoid multi-line ternary; use if/unless instead.

# badsome_condition ? (nested_condition ? nested_something : nested_something_else) : something_else# goodifsome_conditionnested_condition ? nested_something : nested_something_elseelsesomething_elseend

Use &&/|| for boolean expressions, and/or for control flow.

# boolean expressionifdocument.text_changed? || document.settings_changed?document.save!end# control flowdocument.saved?ordocument.save!

Beware: and/or have lower precedence than =!

flag=top_of_page?orreset_page# is equivalent to(flag=top_of_page?)orreset_page

Only use trailing if/unless when they are rare footnotes that can be ignored in the usual, "go-right" case.

That is, the statement you start with should almost always execute. (A good alternative for assertions and other one-line code that rarely executes is control-flow and/or.)

# bad -- the raise rarely executesraiseArgumentError,"name must be provided"unlessname.present?# goodname.present?orraiseArgumentError,"name must be provided"# good -- the unless is a rare footnoteformat(page)unlesspage.already_formatted?# good -- the if is almost always truesend_notification(users)ifusers.any?

Never use unless with else. Rewrite these with the positive case first.

# badunlesssuccess?puts'failure'elseputs'success'end# goodifsuccess?puts'success'elseputs'failure'end

Don't use parentheses around the condition of an if/unless/while, unless the condition contains an assignment.

(see "Using the return value of =" below).

# badif(x > 10)
...
end# goodifx > 10
...
end# okif(x=self.next_value)
...
end

Favor modifier while/until usage when you have a single-line body.

# badwhilesome_conditiondo_somethingend# gooddo_somethingwhilesome_condition

Favor until over while for negative conditions.

# baddo_somethingwhile !some_condition# gooddo_somethinguntilsome_condition

Omit parentheses around parameters for methods that are part of an internal DSL

(e.g. Rake, Rails, RSpec), methods that are with "keyword" status in Ruby (e.g. attr_reader, puts) and attribute access methods. It is preferred to use parentheses around the arguments of all other method invocations.

classPersonattr_reader:name,:age
...
endtemperance=Person.new('Temperance',30)temperance.nameputstemperance.agex=Math.sin(y)array.delete(e)

Prefer {...} over do...end for single-line blocks.

Avoid using {...} for multi-line blocks (multiline chaining is always ugly). Always use do...end for "control flow" and "method definitions" (e.g. in Rakefiles and certain DSLs). Avoid do...end when chaining.

names=['Bozhidar','Steve','Sarah']# goodnames.each{ |name| putsname}# badnames.eachdo |name|
putsnameend# goodnames.select{ |name| name.start_with?('S')}.map{ |name| name.upcase}# badnames.selectdo |name|
name.start_with?('S')end.map{ |name| name.upcase}

Some will argue that multiline chaining would look OK with the use of {...}, but they should ask themselves: is this code really readable and can't the blocks contents be extracted into nifty methods?

Avoid return where not needed for flow of control. Prefer if/else or &&/||.

(Omitting return is more succinct and declarative, and your code will still work if you refactor it into a block later.)

# baddefsome_method(some_arr)returnsome_arr.sizeend# gooddefsome_method(some_arr)some_arr.sizeend# baddefclick_urlreturn"http://#{click_domain}/c"ifclick_domain.nonblank?returnclick_url_for_hostname(vanity_domain)ifvanity_domain.nonblank?click_url_for_hostname(CLICK_URL_DEFAULT_DOMAIN)end# gooddefclick_urlifclick_domain.nonblank?"http://#{click_domain}/c"elsifvanity_domain.nonblank?click_url_for_hostname(vanity_domain)elseclick_url_for_hostname(CLICK_URL_DEFAULT_DOMAIN)endend# baddefshould_redirect?returnfalseunlessrendered?returntrueifredirect_location.nonblank?returntrueifglobal_redirect?returnfalseend# gooddefshould_redirect?
!rendered && (redirect_location.nonblank? || global_redirect?)end# better stilldefhave_redirect?redirect_location.nonblank? || global_redirect?enddefshould_redirect?
!rendered && have_redirect?end

Only use self when required for calling a self write accessor.

# baddefready?ifself.last_reviewed_at > self.last_updated_atself.worker.update(self.content,self.options)self.status=:in_progressendself.status == :verifiedend# gooddefready?iflast_reviewed_at > last_updated_atworker.update(content,options)self.status=:in_progressendstatus == :verifiedend

As a corollary, avoid shadowing methods with local variables unless they are both equivalent.

classFooattr_accessor:options# okdefinitialize(options)self.options=options# both options and self.options are equivalent hereend# baddefdo_something(options={})unlessoptions[:when] == :lateroutput(self.options[:message])endend# gooddefdo_something(params={})unlessparams[:when] == :lateroutput(options[:message])endendend

Use spaces around the = operator when assigning default values to method parameters:

# baddefsome_method(arg1=:default,arg2=nil,arg3=[])# do something...end# gooddefsome_method(arg1=:default,arg2=nil,arg3=[])# do something...end

While several Ruby books suggest the first style, the second is much more prominent in practice (and arguably a bit more readable).

Avoid line continuation (\) unless absolutely required.

# badresult=1 \
- 2# betterresult=1 -
2

Using the return value of = (an assignment) is OK.

But surround the assignment with parentheses to make it clear you are not mistakenly using = when you meant ==.

# good - shows intended use of assignmentif(v=array.grep(/foo/)) ...
# badifv=array.grep(/foo/) ...
# also good - shows intended use of assignment and has correct precedence.if(v=next_value) == 'hello' ...

Don't use ||= to initialize boolean variables.

(Consider what would happen if the current value happened to be false.)

# bad - would set enabled to true even if it was falseenabled ||= true# goodenabled=trueifenabled.nil?

Avoid using Perl-style special variables (like $0-9, `$``, etc. ).

They are cryptic and global.

Never put a space between a method name and the opening parenthesis.

# badf(3 + 2) + 1# goodf(3 + 2) + 1

Ruby 1.9 hash literal syntax is preferred when the hash keys are symbols.

# badhash={:one=>1,:two=>2}# goodhash={one: 1,two: 2}

-> / lambda are preferred over proc/Proc.new.

This is because lambdas enforce argument list cardinality and have unsurprising return semantics. (Only use proc if you really need a return statement that returns from the enclosing code.) More details here.

-> (stabby lambda) syntax is preferred when there are arguments.

This is because stabby lambda arguments are treated just like regular method arguments. For example, these special arguments work for stabby lambda but not lambda or Proc.new:

  • default arguments
  • keyword arguments
  • block (&) argument
# badlam=lambda{ |a,b| a + (b || 0)}lam.call(1,2)# bad - space between stabby lambda and argumentslam=->(a,b=0){a + b}lam.call(1,2)# goodlam=->(a,b=0){a + b}lam.call(1,2)

-> (stabby lambda) syntax is preferred even for multi-line blocks

# badlam=lambdado |a,b|
c=b || 0a + cend# goodlam=->(a,b)doc=b || 0a + cend

Use _ for unused block parameters.

# badresult=hash.map{ |k,v| v + 1}# goodresult=hash.map{ |_,v| v + 1}

Do not rely on a hash being destructured into keyword args, because Ruby 3 will remove this implicit behavior. Instead, use **.

deffoo(a:,b:)# ...endfoo({a: 1,b: 2})# badfoo(a: 1,b: 2)# goodoptions={a: 1,b: 2}foo(options)# badfoo(**options)# good

Use '{}' when passing a hash as an argument for a method that also includes keyword args.

defbaz(options={},c:,d:)# ...endbaz(a: 1,b: 2,c: 3,d: 4)# badbaz({a: 1,b: 2},c: 3,d: 4)# good

Omit {} when passing options hashes at the end of method calls, to enable them to be converted to keyword args in the future without having to change the calling code.

defbar(name,value,options={})# ...endbar("John",10,{a: 1,b: 2})# badbar("John",10,a: 1,b: 2)# good

Naming

Use snake_case for methods and variables.

Use CamelCase for classes and modules. Keep acronyms like HTTP, RFC, XML uppercase when possible. (Upper case words cause problems for rails routes.)

Use SCREAMING_SNAKE_CASE for other constants.

The names of predicate methods (methods that return a boolean value) should end in a question mark.

(e.g. Array#empty?)

The names of potentially "dangerous" or surprising methods should end in an exclamation mark!

(e.g. methods that have side-effects like mutating a variable or changing the process flow)

Prefer

  • map over collect
  • reduce over inject
  • find over detect
  • select over find_all
  • size over length

Company names with capitals in the middle (e.g. RingRevenue) should drop the inner capitalization so that rails string helpers don't insert underscores in the middle

Examples:

  • RingRevenue: Constantize: Ringrevenue, underscored: ringrevenue
  • HubSpot => Hubspot hubspot
  • HubspotIntegration => hubspot_integration

Comments

Good code is its own best documentation. As you're about to add a comment, ask yourself, "How can I improve the code so that this comment isn't needed?" Improve the code and then document it to make it even clearer.
-- Steve McConnell

Enough said.

Classes

Namespace Definition

Define (and reopen) namespaced classes and modules using explicit nesting. Using the scope resolution operator can lead to surprising constant lookups due to Ruby's lexical scoping, which depends on the module nesting at the point of definition.

moduleUtilitiesclassQueueendend# goodmoduleUtilitiesclassWaitingListModule.nesting# => [Utilities::WaitingList, Utilities]definitialize@queue=Queue.new# Refers to Utilities::Queueendendend# badclassUtilities::StoreModule.nesting# => [Utilities::Store]definitialize# Refers to the top level ::Queue class because Utilities isn't in the# current nesting chain.@queue=Queue.newendend

Use @ class variables when you want separate values per subclass.

Use @@ variables when you want process-wide globals (such as for a process-wide cache).

classParent@@class_var='parent'defself.print_class_varputs@@class_varendendclassChild < Parent@@class_var='child'endParent.print_class_var# => will print "child"

As you can see all the classes in a class hierarchy actually share one class variable. Class instance variables should usually be preferred over class variables.

Assign proper visibility levels to methods (private, protected) in accordance with their intended usage.

Indent the public, protected, and private methods as much the method definitions they apply to. Leave one blank line above and below them.

classSomeClassdefpublic_method
...
endprivatedefprivate_method
...
endend

Use class << self or def self.method to define singleton methods so you don't have to repeat the class name (Don't Repeat Yourself).

classTestClass# baddefTestClass.some_method
...
end# gooddefself.some_other_method
...
end# best# this form lets you define many class methods,# and public/private# and attr_reader work as expected.class << selfdeffirst_method
...
endprivatedefsecond_method_etc
...
endendend

SOLID Object Oriented Design

These concepts should be applied generally when writing Object Oriented Code. Read more here.

Single Responsibility Principle in More Detail

"Gather together the things that change for the same reasons. Separate those things that change for different reasons." -- Robert C. Martin

Design your classes such that all public methods relate to a singular purpose. Similarly, define your methods such that each only accomplishes a single task.

TIP: Describe your class aloud and listen for usage of the word "AND" which implies that there are more responsibilities than are necessary.

Examples
# BADclassCorrectionProcessor# Said aloud: "This class processes a correction.. and performs API handling and constructs a custom logger"defprocessif(response=call_third_party_api["response"]) && response["code"] == 200Correction.create!write_to_logs("success!")elsifresponse["code"] == 403write_to_logs("unauthorized")endend# Problem: Unnecessary Public Method makes the purpose of the class uncleardefwrite_to_logs(message)# Problem: Constructing a custom logger might require changes# as soon as we need to improve logging across the application.Log4r::Logger.new("Application Log").info(message)endprivate# Problem: Unencapsulated API behavior -- this class might require changes# as soon as this third party API changes.defcall_third_party_apiresponse=Http.get('www.invoca.net').responseJSON.parse(response)endend

We've extracted logging to a module which can be reused and have established a contract between this class and the module. So long as log_info is maintained, Logging can be freely extended. We also marked log_info as protected, which ensures that future developers are not able to violate the encapsulation of CorrectionProcessor and our Logging module by using CorrectionProcessor for logging for other classes.

# GoodmoduleLoggingprotecteddeflog_info(message)logger.info(message)endprivatedeflogger@logger ||= Log4r::Logger.newendendclassCorrectionProcessor# Said aloud: "This class processes a correction"includeLoggingdefprocessifinvoca_api_call.success?Correction.create!log_info("success!")elsifinvoca_api_call.unauthorized?log_info("unauthorized")endend# continued below..

We've extracted API Handling to a class which simplifies the task of understanding CorrectionProcessor, in doing so, we've established that InvocaApiCall should have public methods (success? and unauthorized?) that CorrectionProcessor can rely on. Future API updates can be accomplished so long as these methods still exist.

# Goodprivatedefinvoca_api_call@invoca_api_call ||= InvocaApiCall.newendendclassInvocaApiCalldefinitialize@response=JSON.parse(perform_api_call)enddefperform_api_callNet::Http.get('www.invoca.net')enddefsuccess?@response["code"] == "200"enddefunauthorized?@response["code"] == "403"endend

Composition and Inheritance

Generally, prefer Composing objects of behavior rather than relying on Inheriting behavior. Inheritance, especially after 2nd and 3rd levels of inheritance, becomes vastly harder to understand as it is extended. However, even composition has it's limitations and its best to apply either with a degree of moderation. (See: Composition Over Inheritance)

Composition Over Inheritance

When faced with at least two (though some would argue three) examples of code reuse, consider extracting that code reuse into a well-named module or object which represents that behavior. See below for examples, and gather feedback from your team early to ensure your abstractions aid in the understanding of the class.

Examples
# BADclassAnimaldefwalkendendclassCat < AnimalendclassDog < AnimalendclassDolphin < Animal# Dolphins probably can't walk! We now need to go back and refactor Animal to account for this case, which can become# very challenging with multiple levels of inheritance!end
# GoodmoduleWalkabledefwalkendendclassCatincludeWalkableendclassDogincludeWalkableendclassDolphinend
# ALSO BAD -- your module should describe the behavior it representsmoduleAnimalHelperdefwalkendendclassCatincludeAnimalHelperendclassDogincludeAnimalHelperendclassDolphinend

Exceptions

Never return from an ensure block.

If you explicitly return from a method inside an ensure block, the return will take precedence over any exception being raised, and the method will return as if no exception had been raised at all. In effect, the exception will be silently thrown away.

deffoobeginfailensurereturn'very bad idea'endend

Use implicit begin blocks when possible.

# baddeffoobegin# main logic goes hererescue# failure handling goes hereendend# gooddeffoo# main logic goes hererescue# failure handling goes hereend

Mitigate the proliferation of begin blocks by using contingency methods (a term coined by Avdi Grimm).

# badbeginsomething_that_might_failrescueIOError# handle IOErrorendbeginsomething_else_that_might_failrescueIOError# handle IOErrorend# gooddefwith_io_error_handlingyieldrescueIOError# handle IOErrorendwith_io_error_handling{something_that_might_fail}with_io_error_handling{something_else_that_might_fail}

Don't suppress exceptions.

# badbegin# an exception occurs hererescueSomeError# the rescue clause does absolutely nothingend# baddo_somethingrescuenil

Don't use exceptions for flow of control.

# badbeginn / drescueZeroDivisionErrorputs'Cannot divide by 0!'end# goodifd.zero?puts'Cannot divide by 0!'elsen / dend

Avoid rescuing the Exception class. This will trap signals and calls to exit, requiring you to kill -9 the process.

# badbegin# calls to exit and kill signals will be caught (except kill -9)exitrescueExceptionputs"you didn't really want to exit, right?"# exception handlingend# goodbegin# a blind rescue rescues from StandardError, not Exception as many# programmers assume.rescue=>e# exception handlingend# also goodbegin# an exception occurs hererescueStandardError=>e# exception handlingend

Put more specific exceptions higher up the rescue chain, otherwise they'll never be rescued from.

# badbegin# some coderescueException=>e# some handlingrescueStandardError=>e# some handlingend# goodbegin# some coderescueStandardError=>e# some handlingrescueException=>e# some handlingend

Release external resources obtained by your program in an ensure block.

f=File.open('testfile')begin# .. processrescue# .. handle errorensuref.closeunlessf.nil?end

Use exceptions from the standard library for simple cases so you can avoid introducing new exception classes.

If you create a custom exception class, always inherit from StandardError not Exception

Collections

Use Set instead of Array when dealing with unique elements.

Set implements a collection of unordered values with no duplicates. This is a hybrid of Array's intuitive inter-operation facilities and Hash's fast lookup.

Avoid the use of mutable object as hash keys.

Rely on the fact that hashes in 1.9 are ordered.

Never modify a collection while traversing it.

Don't use the %w() syntax for defining arrays

Define them as ['a', 'b'] or 'a b'.split(' ')

Strings

Prefer string interpolation instead of string concatenation:

# bademail_with_name=user.name + ' <' + user.email + '>'# goodemail_with_name="#{user.name} <#{user.email}>"

String#<< performs better by mutating the string in place.

String#+, avoids mutation (which is good in a functional way) but therefore runs slower since it creates a new string object.

# fasterhtml=''paragraphs.eachdo |paragraph|
html << "<p>#{paragraph}</p>"end# more functional but slowerhtml=''paragraphs.eachdo |paragraph|
html += "<p>#{paragraph}</p>"end# most functionalhtml=paragraphs.mapdo |paragraph|
"<p>#{paragraph}</p>"end.join

Formatting - Prefer Templated String Formatting

# badformat("%s","Hello")"%<greeting>s" % {greeting: "Hello"}# goodformat("%{greeting}",greeting: "Hello")"%{greeting}, %{user}!" % {greeting: "Hello",name: "User"}

Constants

CamelCase constants should be used only to name classes and modules. ALL_CAPS constants should always be immutable values, and frozen to guarantee that.

# badStdoutLogger=Logger.new(STDOUT)SUPPORTED_VERBS=[:get,:put,:post,:patch,:options]# goodmoduleStdoutLoggerclass << selfdeflogger@logger ||= Logger.new(STDOUT)endendendSUPPORTED_VERBS=[:get,:put,:post,:patch,:options].freeze

Regular Expressions

Don't use a regular expression when a plain string will do:

# badifcommand[/quit/]
...
end# goodifcommand['quit']
...
end

For simple constructions you can use regexp directly through string index.

match=string[/regexp/]# get content of matched regexpfirst_group=string[/text(grp)/,1]# get content of captured groupstring[/text (grp)/,1]='replace'# string => 'text replace'

Avoid using $1-9 as it can be hard to track what they contain and they live globally. Numbered indexes or named groups can be used instead.

# bad/\A(https?):/ =~ url
...
setup_connection($1)# goodprotocol=url[/\A(https?):/,1]
...
setup_connection(protocol)# good/\A(?<protocol>https?):/ =~ url
...
setup_connection(protocol)

Be careful not to use ^ and $ for anchors (like in other languages) because in Ruby they also match newlines.

For anchors, use \A and \z (not to be confused with \Z which is the equivalent of /\n?\z/).

string="some injection\nusername"string[/^username$/]# matchesstring[/\Ausername\z/]# don't match

Use x modifier for complex regexps so that you can use whitespace and comments to make them more readable.

Just be careful as spaces are ignored.

regexp=%r{ start # some text\s # white space char (group) # first group (?:alt1|alt2) # some alternation end}x

For complex replacements sub/gsub can be used with a block or hash.

Metaprogramming

Only use metaprogramming when necessary.

For example, deliver_<mail_message> in TMail was completely unnecessary since it was equivalent to simply deliver(:mail_message).

Do not monkey patch core classes unless you really need to change their behavior.

Generally across all code in the process including other gems and libraries (separation of concerns!):

# badclassFixnumdefdays
...
endend

The block form of class_eval is preferable to the string-interpolated form.

  • when you use the string-interpolated form, always supply __FILE__ and __LINE__, so that your backtraces make sense:
class_eval'def use_relative_model_naming?; true; end',__FILE__,__LINE__
  • define_method is preferable to class_eval { def ... }

Avoid method_missing if possible.

Backtraces become messy; the behavior is not listed in #methods; misspelled method calls might silently work (nukes.launch_state = false). Consider using delegation, proxy, or define_method instead. If you must, use method_missing,

  • be sure to also define respond_to_missing?
  • only catch methods with a well-defined prefix, such as find_by_* -- make your code as assertive as possible.
  • call super at the end of your statement
  • delegate to a reusable, testable, non-magical method:
# baddefmethod_missing?(meth, *args, &block)if/^find_by_(?<prop>.*)/ =~ meth# ... lots of code to do a find_byelsesuperendend# gooddefmethod_missing?(meth, *args, &block)if/^find_by_(?<prop>.*)/ =~ methfind_by(prop, *args, &block)elsesuperendend# best of all, though, would be to define_method as each findable attribute is declared

Testing

Avoid using send to test private methods

Private methods are private for a reason — they represent internal implementation details that should not be relied upon by external code, including tests. Testing private methods directly couples your tests to implementation rather than behavior, making refactoring difficult.

Why this matters:

  • Private methods can change or be removed without breaking the public contract
  • Tests that use send to access private methods become brittle and harder to maintain
  • It violates encapsulation and can lead to over-specified tests
  • It makes it unclear what the actual public API of your class is

What to do instead:

  • Test the public interface of your class—private methods should be tested indirectly through the public methods that use them
  • If a private method is complex enough that you feel it needs direct testing, it's probably a sign that it should be:
    • Extracted into its own class with a public interface
    • Moved to a module or helper that can be tested independently
    • Reconsidered as a protected or public method if other objects legitimately need it
# bad - testing private method directlyclassMyClassdefpublic_methodprivate_methodendprivatedefprivate_method"does something"endend# bad testit"tests private method directly"doobj=MyClass.newexpect(obj.send(:private_method)).toeq("does something")end# good - test through public interfaceit"tests public method behavior"doobj=MyClass.newexpect(obj.public_method).toeq(expected_result)end# better - extract to testable component if complexity warrantsclassCalculatordefself.compute(value:)# complex calculation logicvalue * 1.5endendclassMyClassdefpublic_methodCalculator.compute(value: process_data)endend# now you can test Calculator independentlyit"computes value correctly"doexpect(Calculator.compute(value: 10)).toeq(15)end

When it might be acceptable: In rare cases, you might have a legitimate reason to use send in tests:

  • Testing legacy code that cannot be refactored safely
  • Verifying specific edge cases in private methods during a major refactoring (with the intent to remove after refactoring is complete)
  • Testing framework internals where you're explicitly testing private implementation

If you find yourself in one of these situations, document why with a clear comment and consider whether there's a better approach.

Misc

Write ruby -w safe code when practical.

Try an editor that will show you these when you save.

When code patterns are repeated, use separate lines and extra whitespace when practical to align columns so that the code is tabular.

This makes the patterns obvious which helps to spot/prevent bugs.

# badPROMOTIONAL_METHODS=[['review_site','Content / Review Site'],['coupon_site','Discount / Coupon Site'],['display','Display'],['email','Email'],['rewards','Rewards / Incentive'],['leads','Lead Form / Co Reg'],['search','Search'],['social_media','Social Media'],['software','Software'],['other','Other']],
...
# goodPROMOTIONAL_METHODS=[['review_site','Content / Review Site'],['coupon_site','Discount / Coupon Site'],['display','Display'],['email','Email'],['rewards','Rewards / Incentive'],['leads','Lead Form / Co Reg'],['search','Search'],['social_media','Social Media'],['software','Software'],['other','Other']],
...
# badparams={'phone'=>calling_phone_number.to_param,'LastName'=>last_name,'FirstName'=>first_name,'Address'=>primary_address,'ApartmentNum'=>secondary_address,'City'=>city_name,'State'=>state,'Zipcode'=>zip}
...
# goodparams={'phone'=>calling_phone_number.to_param,'LastName'=>last_name,'FirstName'=>first_name,'Address'=>primary_address,'ApartmentNum'=>secondary_address,'City'=>city_name,'State'=>state,'Zipcode'=>zip}

Prefer code written in the functional style

avoid side-effects like object mutation unless required by performance concerns. Mutating arguments is a side-effect so don't do it unless that is the sole purpose of the method.

  • Don't put required parameters into options hashes.

Try to keep methods to 10 lines of code or less.

Ideally, most methods will be shorter than 5 lines of code. Comments and empty lines do not count.

Try to keep parameter lists limited to three or four parameters.

Avoid alias when alias_method will do.

Use OptionParser for parsing complex command line options and

ruby -s for trivial command line options.

Preferred Ruby-isms

Items under this section are designed to help clarify design patterns common in our Ruby code. These patterns are designed to help in various ways, including but not limited to:

  1. Avoiding common bugs
  2. Creating code that is more:
    • maintainable
    • understandable
    • extendable
    • intention revealing

Casting Booleans with !!

A common ruby-ism for casting booleans is to precede the method call(s) with !!. By calling the negation operator twice, we cast to a boolean twice. The first is the negated boolean (e.g. nil => true) and the second converts the new boolean to the original truthiness (e.g. nil => true => false).

This style should be used when truthiness is not acceptable, and a true or false value is required or expected.

Useful
# JSON expects true/false, not truthinessdefto_jsonrecord=find_by_id(1){record_exists: !!record}end
Unnecessary
# The if only needs truthiness, not explicit true or falseif !!User.find_by_id(1)
...
end