rails-style-guide icon indicating copy to clipboard operation
rails-style-guide copied to clipboard

Cop idea: merge `.first` || `.create!` into `.first_or_create!`

Open ydakuka opened this issue 1 year ago • 2 comments

# bad
def method
  users.where(name: name).first ||
    users.where(name: name).create!
end

# bad
def method
  users.where(name: name).first ||
    users.create!(name: name)
end

# good
def method
  users.where(name: name).first_or_create!
end

And for similar methods like find_or_create_by, find_or_create_by!, find_or_initialize_by, first_or_create, first_or_create!, first_or_initialize, create_or_find_by and create_or_find_by!

ydakuka avatar Oct 20 '23 17:10 ydakuka

This is a matter of style. Before deciding, let's discuss it in the style guide.

koic avatar Oct 20 '23 18:10 koic

The "good" style is preferable to me, because it helps me organize my code better.

Compare with this example (from my project):

def hsh
  { name: name }
end
# before

def method1
  users.where(hsh).first || users.create!(hsh)
end

def method2
  users.where(hsh).first || users.where(hsh.merge(premium: true)).create!
end
# after

def method1
  users.where(hsh).first_or_create!
end

def method2
  users.where(hsh).first || users.where(hsh.merge(premium: true)).create!
end

ydakuka avatar Oct 21 '23 14:10 ydakuka

Isn't first_or_create deprecated? https://github.com/rails/rails/pull/29584#issuecomment-311427304

Linuus avatar May 06 '24 09:05 Linuus

@Linuus Good catch. I'll close this proposal.

koic avatar May 06 '24 09:05 koic

Maybe we should add a cop that warns when first_or_create is used instead 😬

Linuus avatar May 07 '24 08:05 Linuus

Let’s leave it out to the Rails team to print deprecation warnings.

pirj avatar May 07 '24 11:05 pirj