Il pattern visitatore viola l'SRP?

Il modello di visitatore classico non viola necessariamente SRP. Tuttavia, il codice mostrato nella domanda fa. Se si ha solo bisogno di un'interfaccia "per poter passare la struttura dei dati di contribuzione a" , perché non implementare questo class C401k (aggiunto C per far sì che il nome della classe non inizi con una cifra) come esattamente questo, e non più ? La classe potrebbe avere questo aspetto:

class C401k
{
  public void MakeContribution(Contribution contribution)
  {
      contributions.Add(contribution);
  }
  public IContributions GetContributions()
  {
      return contributions;
  }

  private IContributions contributions = new ContributionsList();

  // no report generatiom or balance calculation code here!
}

Ora, il tuo metodo CalculateBalance deve apparire come questo

 public Money CalculateBalance(I401kBalanceCalculator balanceCalculator,
                               C401k contribs401k)
 {
    // ... maybe some more logic here
    return balanceCalculator.Calculate(contribs401k.GetContributions());
 }

che porta alla domanda dove mettere questo metodo. Un possibile luogo potrebbe essere C401k , ma in questo esempio, con un punto di ingresso di generazione di report e un punto di ingresso di calcolo in una classe e un nome molto vago come 401k che non descrive una chiara responsabilità, c'è IMHO un certo rischio di creare una classe di Dio che contiene molti metodi di business logic diversi, non correlati. Ciò violerebbe l'SRP. Vedo altri due posti candidati qui:

• se CalculateBalance non contiene molto più logica di quella che vediamo qui, (poiché tutta la logica è da qualche parte in I401kBalanceCalculator ), potrebbe essere un metodo della classe che lo chiama. Se non è necessario più di una volta, e non c'è più logica di quella mostrata nell'esempio, si potrebbe anche considerare di rimuovere completamente il metodo e sostituire la sua chiamata semplicemente con la riga balanceCalculator.Calculate(contribs401k.GetContributions()) .

• o, se nel codice reale, c'è più logica in CalculateBalance rispetto a questo esempio forzato, ha probabilmente senso metterlo in una classe controller separata, qualcosa come un BalanceCalculatorController . Il controller potrebbe quindi avere questo aspetto:

  class BalanceCalculatorController
  {
     public BalanceCalculatorController(I401kBalanceCalculator balanceCalculator, 
                                        C401k contribs401k)
     {
        this.balanceCalculator=balanceCalculator;
        this.contribs401k=contribs401k;
     }
     public Money Calculate()
     {
         // ... implement additional logic here ...
         return balanceCalculator.Calculate(contribs401k.GetContributions());
     }
  
     private // some helper methods here ...
  }
  

Naturalmente, bisognerà pensare a dove creare questo controller, dove passarlo per riutilizzarlo, come testarlo e così via. Quindi questo rende IMHO solo senso se c'è davvero più logica lì che merita una classe a sé stante.

Does visitor pattern lead to violation of SRP?

Penso che siano generalmente concetti ortogonali. Ma è sicuramente una violazione del concetto forse più importante - incapsulamento .

La prima riga in wikipedia dice:

the visitor design pattern is a way of separating an algorithm from an object structure on which it operates.

E questo è un tratto vivido della programmazione procedurale, in contraddizione con i principi di base dell'OOP. Quindi, secondo me, la logica di calcolo dell'equilibrio dovrebbe risiedere nella classe 401k. Tuttavia, ciò non significa che non puoi usare altre classi per aiutarti. Puoi, ma senza esporre il comportamento interno di 401k .

Ma c'è un problema di 401k che diventa un oggetto di Dio, menzionato da Doc Brown. Vorrei dare un'occhiata se avete bisogno di un oggetto 401k che rappresenta la stessa classe in tutti quei casi d'uso in cui vengono chiamati i suoi metodi attuali. Prima di tutto, considera i tuoi contesti limitati. Dubito che la stessa classe debba rappresentare un caso d'uso per aggiungere un contributo e generare un rapporto (inoltre, dubito che abbiate mai bisogno di un oggetto quando tutto ciò di cui avete bisogno è la rappresentazione dei dati). Le probabilità sono che appartengono a diversi BC. Di conseguenza, puoi dividere il modello sul lato di scrittura e lettura. Aggiungere un contributo è un modello di scrittura, il tuo dominio, mentre un calcolo del saldo probabilmente potrebbe essere il tuo read model .

Ecco come potrebbe essere il repository del rapporto:

class C401kReportRepository
{
    public function report()
    {
        // query your database
        return
            [
                [
                    'year' => 2017,
                    'month' => 11,
                    'amount' => 10000,
                ]
            ];
    }
}

La tua ContributionsList può essere esposta solo se stai utilizzando un ORM ( che non è obbligatorio , a proposito), e questo può essere fatto implicitamente dal framework stesso, usando la riflessione, per esempio. Il punto non è esporlo a clienti arbitrari. Quindi se i dati di ContributionsList vengono utilizzati all'interno di un certo insieme di oggetti ( 401k oggetto nel tuo caso), non è un grosso problema. Dopo tutto, i tuoi dati devono essere esposti da qualche parte. Basta non renderlo un'esposizione di default, con i getter.