rubocop / rails-style-guide

A community-driven Ruby on Rails style guide
http://rails.rubystyle.guide
6.48k stars 1.06k forks source link

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

Closed ydakuka closed 7 months ago

ydakuka commented 1 year ago
# 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!

koic commented 1 year ago

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

ydakuka commented 1 year ago

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
Linuus commented 7 months ago

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

koic commented 7 months ago

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

Linuus commented 7 months ago

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

pirj commented 7 months ago

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