Ruby

# ❌ Manual accumulation
result = []
items.each do |item|
  result << item.name.upcase if item.active?
end

# ✅
result = items.select(&:active?).map { |i| i.name.upcase }

# ❌ Manual grouping
grouped = {}
items.each do |item|
  grouped[item.category] ||= []
  grouped[item.category] << item
end

# ✅
grouped = items.group_by(&:category)

# ❌ Manual sum
total = 0
orders.each { |o| total += o.amount }

# ✅
total = orders.sum(&:amount)

# ❌ Checking existence then accessing
if hash.key?(key)
  value = hash[key]
end

# ✅
value = hash[key] # returns nil if missing
# or with default:
value = hash.fetch(key, default_value)
# or raising on missing:
value = hash.fetch(key) # raises KeyError

# ❌ Explicit block-to-proc conversion when unnecessary
items.map { |item| item.to_s }

# ✅
items.map(&:to_s)

# ❌ Proc.new when lambda is safer (arity check + return behavior)
handler = Proc.new { |x| x * 2 }

# ✅
handler = ->(x) { x * 2 }

# ❌ Multi-line block with { }
items.map { |item|
  result = transform(item)
  validate(result)
  result
}

# ✅ — do/end for multi-line, { } for single-line
items.map do |item|
  result = transform(item)
  validate(result)
  result
end

# ❌ String concatenation in loop
result = ""
items.each { |i| result += i.name + ", " }

# ✅
result = items.map(&:name).join(", ")

# ❌ String concatenation for assembly
greeting = "Hello, " + name + "! You have " + count.to_s + " messages."

# ✅
greeting = "Hello, #{name}! You have #{count} messages."

# ❌ Mutable string where frozen is fine (Ruby 3+ encourages frozen)
SEPARATOR = ", "

# ✅
SEPARATOR = ", ".freeze
# or add `# frozen_string_literal: true` at file top

# ❌ Rescuing Exception (catches EVERYTHING including SignalException, SystemExit)
begin
  risky
rescue Exception => e
  log(e)
end

# ✅ — rescue StandardError (the default)
begin
  risky
rescue StandardError => e
  log(e)
  raise
end
# or just: rescue => e (same as StandardError)

# ❌ Using rescue as flow control
begin
  value = hash.fetch(key)
rescue KeyError
  value = default
end

# ✅
value = hash.fetch(key, default)

# ❌ Inline rescue hiding errors
result = dangerous_operation rescue nil

# ✅ — inline rescue only for truly trivial fallbacks
result = Integer(input) rescue nil  # acceptable for parsing

# ❌ Manual accessors
class User
  def name
    @name
  end
  def name=(value)
    @name = value
  end
end

# ✅
class User
  attr_accessor :name
end

# ❌ Deep inheritance for shared behavior
class Animal; end
class Pet < Animal; end
class Dog < Pet; end

# ✅ — mixins for shared behavior, inheritance for "is-a"
module Trainable
  def train = puts("Training #{name}")
end

class Dog
  include Trainable
  attr_reader :name
  def initialize(name) = @name = name
end

# ❌ Class with only class methods (namespace via class)
class MathUtils
  def self.square(x) = x * x
  def self.cube(x) = x ** 3
end

# ✅
module MathUtils
  module_function
  def square(x) = x * x
  def cube(x) = x ** 3
end

# ❌ Explicit boolean return
def active?
  if status == :active
    true
  else
    false
  end
end

# ✅
def active? = status == :active

# ❌ nil check before method call
if user && user.name
  puts user.name
end

# ✅ (Ruby 2.3+)
puts user&.name if user&.name
# or with safe navigation:
user&.name&.then { |n| puts n }

# ❌ Conditional assignment verbosely
if @cache.nil?
  @cache = expensive_compute
end

# ✅
@cache ||= expensive_compute

# ❌ Multiple assignment from array manually
first = arr[0]
second = arr[1]

# ✅
first, second = arr