Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I consider this two options exactly the same. I prefer the first one but I'm not sure if it's better the second option in terms of cyclomatic complexity or readability.

Assuming that this is not pure Ruby and I can call to the try method.

existing_transaction = database.find_transaction_by_id(transaction.id)
database.create_transaction(transaction) unless existing_transaction
database.update_transaction(transaction) unless existing_transaction.try(:processed)

existing_transaction = database.find_transaction_by_id(transaction.id)
if existing_transaction
  unless existing_transaction.processed
    database.update_transaction(transaction)
  end
else
  database.create_transaction(transaction)
end
share|improve this question

2 Answers 2

up vote 4 down vote accepted

It's obviously subjective, but IMHO inline conditionals make code harder to understand, indentation is very important. So I'd definitely take the second approach, but writing it differently to use only one indentation level:

existing_transaction = database.find_transaction_by_id(transaction.id)
if !existing_transaction
  database.create_transaction(transaction)
elsif !existing_transaction.processed
  database.update_transaction(transaction)
end
share|improve this answer

I would actually take a mixed approach:

if existing_transaction
  database.update_transaction(transaction) unless existing_transaction.processed
else
  database.create_transaction(transaction)
end

This way the code is more readable than the first option, but more succinct than the second option.

share|improve this answer
    
This is exactly my solution wrote with unless inlined. –  Arturo Herrero Mar 15 at 19:57
    
Yes, it is - it just reads better IMHO –  Uri Agassi Mar 15 at 20:18

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.