Methods should do one thing - one question

[wowremywang]

Hi @uohzxela Thanks for your good contents. It is really helpful. But I have a question about Methods should do one thing

Bad:

def email_clients(clients)
  clients.each do |client|
    client_record = database.lookup(client)
    email(client) if client_record.active?
  end
end

email_clients(clients)

Good:

def email_clients(clients)
  clients.each { |client| email(client) }
end

def active_clients(clients)
  clients.select { |client| active_client?(client) }
end

def active_client?(client)
  client_record = database.lookup(client)
  client_record.active?
end

email_clients(active_clients(clients))

It seems Good code is more clear but if we use this code, we have to loop all clients twice. But first Bad code only loops clients once. So Bad code is faster than Good code. What do you think?

[gathuku]

Am thinking this is better too