Mettre un lien vers le site et/ou les profils de Sandi ou Katrina ce serait cool. Ah mais c'est à la toute fin de l'article 💡 ! D'ailleurs il y a une typo, c'est Sandi avec un "i".
Plus de dépendances ! J'ai souvenir d'autre type de couplage/dépendance :
- La dépendance temporelle : A doit être exécuté avant B
- C'est de la mauvaise encapsulation
- La dépendance à une API : A connait la signature de B (nom de méthode, nombre d'arguments, ...)
- C'est plus sournois et l'inversion de controle peut être une solution
C'est pas un peu étrange ce modèle au pluriel : app/models/user_expenses.rb
?
Je ne fais plus de Rails mais dans mon souvenir les modèles étaient plutôt au singulier.
Ce serait chouette d'expliquer que la classe CSVExporter
sert à extraire la logique d'export du modèle.
def export_expenses(user_expenses)
# not relevant here
end
Super relevant à mon avis car export_expenses
depends du modèle user_expenses
. C'est important car on voit bien que si on ne peut pas se débarrasser d'une dépendence, on peut toujours en limiter la surface. C'est peut être un sujet pour un autre article ceci dit.
Très bien ce allow_any_instance_of(CSVExporter)
, ça souligne un symptome caractéristique d'un excès de dépendences.
Il faut donc se poser la question de savoir quelles informations sont vraiment nécessaires pour le fonctionnement de la classe UserExpenses.
Tout à fait. Pour moi, UserExpenses#export
est un faux ami. Sous couvert de rendre le code plus simple on introduit une dépendence fictive. Un client peut tout à fait utiliser CSVExporter
directement, ça supprimerait complètement cette responsabilité de la class UserExpenses
et donc la dépendence qui va avec:
user_expenses = UserExpenses.find_by(user_id: params[:user_id])
send_data CSVExporter.new.export_expenses(user_expenses)
Une approche pour diminuer les dépendences c'est aussi de laisser la coordination au code appelant.
Si vraiment le code d'orchestration est trop complexe, on peut toujours considérer que c'est un objet à part entière:
class UserExpensesExport
def initialize(user_id)
@user_id = user_id
end
def export
user_expenses = UserExpenses.find_by(@user_id)
CSVExporter.new.export_expenses(user_expenses)
end
end
Il manque un petit .new
sur cette ligne:
when :json then JSONExporter
Il n'y a plus les csv_cols
dans le dernier exemple. Ça montre que csv_cols
ne colle pas bien avec la notion de JSON.
In fine sur le code de l'exemple, j'aurai tendance à préférer :
class UserExpensesReport
SUPPORTED_FORMATS = {
"csv" => CSVExporter,
"json" => JSONExporter,
}
def initialize(user_id:)
@user_id = user_id
end
def build(format:, fields: nil)
user_expenses = UserExpenses.find_by!(user_id: @user_id)
exporter = SUPPORTED_FORMATS.fetch(format).new(fields: fields)
exporter.export_expenses(user_expenses)
end
end
# in app/controllers/exports_controller.rb
class ExportsController
def create
user = User.find(params.fetch(:user_id))
user_expenses_export = UserExpensesExport.new(user_id: user.id)
user_expenses_export_data = user_expenses_export.build(
format: params.fetch(:format, "csv"),
fields: user.custom_export_fields,
)
send_data user_expenses_export_data
end
end
Il est possible d'utiliser stub_const
dans les tests pour injecter un fake dans SUPPORTED_FORMATS
.
Très bonne idée, je vais ajouter des liens vers leur sites / profils et corriger la typo
C’est pas impossible en effet mais ce sont les seules listées sur Wikipedia. Ça pourrait valoir le coup de les ajouter à la liste, la page sur l’injection de dépendance étant tellement pauvre sur la version FR comparait à la version EN.
Par rapport à tes autres retours, j’ai parfaitement conscience que mes exemples ne sont pas parfaits mais j’ai cherché à m’éloigner du livre 99 Bottles of OOP où les exemples sont un peu hors sol par rapport à ce que j’ai l’habitude de faire tout les jours. J’ai donc cherché à trouver un sujet qui corresponde plus à ce que l’on peut faire, je pense qu’on a tous eu (ou alors ça arrivera) un ticket à implémenter concernant un export de données en CSV/JSON.
Effectivement, les models liés à une table en BDD sont toujours nommés au singulier. Ici dans mon idée le model
UserExpenses
et un simple PORO qui représente l’ensemble des dépenses d’un utilisateur (plus pourquoi pas d’autres informations).Je ne trouvais pas ça forcément très utile pour le sujet de l’article, j’avais peur que trop d’informations annexes auraient pus diluer mes propos.
Oui c’est exactement ce que prône l’inversion de dépendance mais je ne voulais pas aborder le sujet tout de suite, pour ne pas balancer trop d’informations. Et qui sait j’en ferais peut-être un article donc il faut garder du click pour plus tard 😄. J’ai donc préféré le montrer dans les derniers exemples mais sans insister dessus (car je sais pas encore si j’arriverais à bien le présenter).
Justement non ici la classe
JSONExporter
est définie comme un module, on peut donc appeler la méthodeexport_expenses
directement car en Ruby tout est objet. Et ça m’a permis aussi de montrer que tout ce que la classeUserExpenses
a besoin est d’un objet (peu importe sa nature) qui réponde à la méthodeexport_expenses
Exact, dans mon idée le dernier exemple se déroule dans un univers parallèle (dsl je viens de voir le trailer de Doctor Strange 2 😄 ) où la fonctionnalité concernant les csv_cols n’a pas été implémenté. Mais je comprends que ça peut-être confusant je vais tâcher de corriger cette partie.
Et moi aussi 😛 excepté que dans cette exemple la classe
UserExpensesReport
se retrouve à créer et utiliser un exporter créant une dépendance sur la méthodeexport_expenses
ET sur la méthodenew
. Ce qui fait qu’il n’est pas possible de modifier simplement le constructeur d’un des exporter car cela va impacter notre report, est-ce qu’il faut modifier le constructeur de l’autre exporter aussi même si il n’a pas besoin de ce nouveau paramètre ? J’aurais tendance à dire non, et donc ça signifierai qu’il faudrait ajouter une condition dans la classeUserExpensesReport
pour ajouter le paramètre pour le format spécial. Et à ce moment là est-ce que c’est vraiment le travail de cette classe là de s’occuper de cette question ? Suivant le contexte je pourrais avoir tendance à préférer l’injection de dépendance qui permet au report de disposer d’un objet prêt-à-être utiliserMais je le répète, j’ai essayé de garder un exemple simple et accessible à tous (y compris les non-rubyists) mais qui reste proche du quotidien quand même