-
-
Save lucasdavila/1785732 to your computer and use it in GitHub Desktop.
# sem linhas em branco, código não tão junto | |
if current_user.has_role('admin') | |
cases = cases.of_product(product) | |
elif current_user.has_role('employee') | |
cases = cases.of_product_accessible_by_employee(product, current_user) | |
elif current_user.has_role('customer') | |
cases = cases.of_product_accessible_by_customer(product, current_user) |
# sem linhas em branco, tudo muito junto | |
if current_user.role? :admin | |
@cases = @cases.of_product @product | |
elsif current_user.role? :employee | |
@cases = @cases.of_product_accessible_by_employee @product, current_user | |
elsif current_user.role? :customer | |
@cases = @cases.of_product_accessible_by_customer @product, current_user | |
end | |
# com linhas em branco, muitas linhas | |
if current_user.role? :admin | |
@cases = @cases.of_product @product | |
elsif current_user.role? :employee | |
@cases = @cases.of_product_accessible_by_employee @product, current_user | |
elsif current_user.role? :customer | |
@cases = @cases.of_product_accessible_by_customer @product, current_user | |
end |
@tarsis uma vez no fisl escutei isso também não use ifs... tentei entender... cheguei a conclusão que a unica maneira de usar menos ifs seria usando objetos, assim cada objeto terá o seu comportamento para o método chamado sem a necessidade de tantos ifs.
É isso mesmo? Alguma sugestão para refatorar este código? Com certeza usar menos ifs deixaria este código menor e mais claro, eliminando a reclamação :P
@lucasdavila vc poderia colocar o metodo dentro do objeto User e deixar que cada instancia saiba o que fazer atraves do polimorfismo. mas a ideia é essa, nao que os ifs sejam ruins, mas muitos aninhados é um sinal de que algo esta errado
Isso é esquisito em qualquer linguagem. Que tal:
@case = case.product_for current_user.role
# ...
def product_for(role)
send ("product_for_" role.to_s).to_s
end
def product_for_admin; [One, Two]; end
def product_for_employee; [Three]; end
# etc
to_sym*
@kurko baseado na tua sugestão, eu poderia fazer assim:
# no model
scope :for_role, lambda { | role, product, user = nil |
send "for_#{role}".to_sym, product, user
}
scope :for_admin, lambda { | product, *args |
...
}
scope :for_employee, lambda { | product, user |
...
}
# e no controlador chamar
@cases = Case.scoped
@cases = @cases.for_role :admin, @product
# ou
@cases = @cases.for_role :employee, @product, current_user
# ou
@cases = @cases.for_role :customer, @product, current_user
# Ainda assim precisaria perguntar a role do user, ai poderia fazer:
@cases = @cases.for_role current_user.role, @product, current_user
# logo o escopo poderia ser refatorado para:
@cases = @cases.for_user current_user, @product #la no escopo poderia perguntar a role do user com user.role
Ainda assim, lá dentro da classe do User (no metodo .role ) precisaria ver quais os papeis do usuário, ai para não usar if lá, a sugestão do @tarsis com polimorfismo se aplica :)
rss de uma insatisfação aprendi algumas coisas xD
@lucasdavila eu faria exatamente como tu fez, a única diferença seria os parenteses, que geralmente eu coloco
if current_user.role?(:admin)
@cases = @cases.of_product(@product)
elsif current_user.role?(:employee)
@cases = @cases.of_product_accessible_by_employee(@product, current_user)
elsif current_user.role?(:customer)
@cases = @cases.of_product_accessible_by_customer(@product, current_user)
end
outra solução é partir para o case
case current_user.role
when :admin
@cases = @cases.of_product(@product)
when :employee
@cases = @cases.of_product_accessible_by_employee(@product, current_user)
when :customer
@cases = @cases.of_product_accessible_by_customer(@product, current_user)
end
ou
@cases = case current_user.role
when :admin
@cases.of_product(@product)
when :employee
@cases.of_product_accessible_by_employee(@product, current_user)
when :customer
@cases.of_product_accessible_by_customer(@product, current_user)
end
que nesse caso eu acho bem mais legível
@rafaelss deixarei como antes, até que pense uma maneira melhor.
if current_user.role? :admin
@cases = @cases.of_product @product
elsif current_user.role? :employee
@cases = @cases.of_product_accessible_by_employee @product, current_user
elsif current_user.role? :customer
@cases = @cases.of_product_accessible_by_customer @product, current_user
end
@rafaelss como em ruby uso 2 espaçamentos, prefiro adicionar linhas em branco como no exemplo para separar visualmente... mas ai um método que era para ser pequeninho fica com várias linhas :/
Tu usa quantos espaçamentos? Costuma usar linhas em branco assim?