style is the only thing.

a.rb

# shitty 


   attributes[:name] = options[:name] unless options[:name].blank?
    attributes[:first_name] = options[:first_name] unless options[:first_name].blank?
    attributes[:last_name] = options[:last_name] unless options[:last_name].blank?
    attributes[:linkedin_id] = options[:linkedin_id] unless options[:linkedin_id].blank?
    attributes[:company] = options[:company] unless options[:company].blank?
    attributes[:company_id] = options[:company_id] unless options[:company_id].blank?
    attributes[:linkedin_picture_url] = options[:linkedin_picture_url] unless options[:linkedin_picture_url].blank?



# shitty, but i can read it...

    attributes[:name]                 = options[:name]                 unless options[:name].blank?
    attributes[:first_name]           = options[:first_name]           unless options[:first_name].blank?
    attributes[:last_name]            = options[:last_name]            unless options[:last_name].blank?
    attributes[:linkedin_id]          = options[:linkedin_id]          unless options[:linkedin_id].blank?
    attributes[:company]              = options[:company]              unless options[:company].blank?
    attributes[:company_id]           = options[:company_id]           unless options[:company_id].blank?
    attributes[:linkedin_picture_url] = options[:linkedin_picture_url] unless options[:linkedin_picture_url].blank?



# at least it's DRY, but fuck it's hard to read... 

   [:name, :first_name, :last_name, :linkedin_id, :company, :company_id, :linkedin_picture_url].each do |key|
     attributes[key] = options[key] unless options[key].blank?
   end



# minimum required to be good 
#
# * the variable name makes the intention clear
# * the list isn't jammed onto one line hiding typos and confounding quick editing
# * it's primed for configuration (whitelist is an option)
# * stack errors come from *one line*
# * it separates the logic of listing what to whitelist and how to extract a key from options
# * reading a commit diff is actually possible (thanks @drbrain)
#


    whitelist = [ 
      :name,
      :first_name, 
      :last_name,
      :linkedin_id,
      :company,
      :company_id,
      :linkedin_picture_url
    ]
    
    whitelist.each do |key|
      attributes[key] = options[key] unless options[key].blank?
    end

What about

    # whitelist
    [ 
      :name,
      :first_name, 
      :last_name,
      :linkedin_id,
      :company,
      :company_id,
      :linkedin_picture_url
    ].each do |key|
      attributes[key] = options[key] unless options[key].blank?
    end

@steveklabnik comments are for the fugly or complicated code. it's extremely likely that whitelist will need to be used elsewhere, be made configurable, need to be inspected in 'pry'. the worse thing about comments is that the next guy to edit the file will delete them - since they may or may not apply: people think nothing about changing an annotation, but will ponder variable naming for 15 minutes (especially intra function comments as opposed to ones dedicated for rdoc, etc)

but that is the next best.

For one-time-use values, I prefer @steveklabnik 's style. But instead of putting a comment on it, I would give a meaningful name to the block argument, like this:

[ 
  :name,
  :first_name, 
  :last_name,
  :linkedin_id,
  :company,
  :company_id,
  :linkedin_picture_url
].each do |accessable_attr|
  attributes[accessable_attr] = options[accessable_attr] if options[accessable_attr].present?
end

I personally would write this without the comment, but added it here to address your 'variable gives a good name' concern.

In 'real' code, I might actually do

   def whitelist
     %s[
      name
      ...
   end

If I planned on using it elsewhere in the class. Then it'd end up being almost the same. But really, without any context here, it's impossible to say what's 'best.'

Except #1 is clearly terrible.

for sure coding is a process. hopefully people are actively moving from the top of that gist somewhere near the bottom. it's okay to not always start there or to take a different route - the motivation to get there is almost as important as getting there; since it's unlikely we ever will...

👍

@drbrain pointed out that an additional advantage is that the last example diffs well in a commit

I will also point out that the Ruby parser allows a comma after the last item in an Array literal (or a Hash) such as:

    whitelist = [ 
      :name,
      :first_name, 
      :last_name,
      :linkedin_id,
      :company,
      :company_id,
      :linkedin_picture_url,
    ]

This has the advantage of having a single-line diff when the next whitelist item is added. The alternative is to have a patch like:

       :company_id,
-      :linkedin_picture_url
+      :linkedin_picture_url,
+      :pet_name
     ]

Rather than simply:

       :linkedin_picture_url,
+      :pet_name,
     ]

i am a huge fan of 'trailing comma' for that very reason. however, years ago matz threatened to make it go away in 1.9.ish.... can anyone confirm otherwise so as to advocate resumed usage?

Well, for Hash at least, the Ruby Spec project codifies the "hanging comma":

https://github.com/rubyspec/rubyspec/blob/master/language/hash_spec.rb#L48

And similar specs for a hanging comma at the end of an argument list for send:

https://github.com/rubyspec/rubyspec/blob/master/language/versions/send_1.9.rb

I don't see anything equivalent for Array unfortunately. However, there does seem to be support in the grammar parse.y for it:

(excerpted from four locations in the file)

primary     : literal
        | tLBRACK aref_args ']'
            {
            /*%%%*/
            if ($2 == 0) {
                $$ = NEW_ZARRAY(); /* zero length array*/
            }
            else {
                $$ = $2;
            }
            /*%
            $$ = dispatch1(array, escape_Qundef($2));
            %*/
            }
aref_args   : none
        | args trailer
            {
            $$ = $1;
            }
        | args ',' assocs trailer
            {
            /*%%%*/
            $$ = arg_append($1, NEW_HASH($3));
            /*%
            $$ = arg_add_assocs($1, $3);
            %*/
            }
        | assocs trailer
            {
            /*%%%*/
            $$ = NEW_LIST(NEW_HASH($1));
            /*%
            $$ = arg_add_assocs(arg_new(), $1);
            %*/
            }
        ;

trailer     : /* none */
        | '\n'
        | ','
        ;