Some comments on your code:
- Maximum line length: 80-100 is ok, more is unreadable.
self[:active] == false
->!self[:active]
- Checks should be terser, encapsulate login in separate methods.
return true
: areturn
at the end of a code block is neither necessary nor idiomatic.- Subjective: I usually prefer full-fledge conditionals to guards, indentation helps a lot to see what's going on, IMO.
I'd write:
def active?
case
when (!self[:active] && !ignore_column) || suspended || completed_at
false
when failed_attempts >= MAX_FAILED_ATTEMPTS
if suppress_error_emails
false
end
message = "Too many failed attempts for task execution. " +
"#{failed_attempts}/#{MAX_FAILED_ATTEMPTS} failed attempts for Task #{id}."
SystemMailer.error(message: message)
end
# ...
# more conditions here, you get the idea
# ...
when template &&
(status_opted_out? || con_mktg_opted_out? || user_mktg_opted_out?)
false
else
true
end
end
Note that that you could go all the way and abstract all logic to methods. Then you could write:
def active?
condition1? && condition2? && ... && conditionN?
end