nicoolas25/NOTES.md Secret

## NOTES.md

      
    Raw
  

              NOTES.md
            
          
    Réflexions sur l’injection de dépendance

Article de Jérémy Bertrand

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.