Recentemente ho eseguito il debug di un po 'di codice in cui l'implementazione precedente era simile a questa:
# controller for group/customers
def index
@customers = current_user.available_customers(param[:group_id].to_i)
end
# user
def available_customers(group_id)
# some other code, that sometimes returns early
# this is the line of failure
# self.accessible_group_ids returns an array of integers
# as does Group.children_ids
# This line is attempting to get the subset of group ids that the user
# is *allowed* to touch, and the hierarchy of group ids under the selected one.
ids = (self.accessible_group_ids & ([group_id] + Group.children_ids(group_id)))
return Customer.where(group_id: ids)
end
Stavo refactoring il metodo di chiamata a available_customers nel controller e ho perso la conversione specifica di params[:group_id].to_i , quindi introducendo un bug in cui quando l'input dell'utente viene inviato tramite i parametri, è una stringa e il confronto nella ids = line non funziona come previsto - esclude il group_id corrente dal set, perché una stringa non è un int, anche se ha le stesse cifre in esso. :)
Vedo questa implementazione originale come ... problematica ... tralasciando le dipendenze dalle classi Group e Customer. È improbabile che la conversione del tipo prima di passare i dati al metodo sia destinata a fallire (come nel mio caso) e richiede al callee di conoscere come funziona il metodo (e anche una conoscenza abbastanza intima!).
Non penso che il chiamato debba sapere che il metodo richiede un int, e in un linguaggio vagamente tipizzato come Ruby, va bene accettare tutti i tipi di anatre negli argomenti del metodo a patto che facciano il ciarlatano.
La mia soluzione era:
def available_customers(group_id)
# nil handling
group_id = group_id.to_i
# everything else
end
Ora il metodo available_customers è l'unico posto che deve sapere che group_id deve essere un numero intero.
Dove pensi che sia un buon posto dove metterlo? Lo metteresti da qualche altra parte? Perché?