I campi del campo di impostazione sono in un sottomodello senza passare un argomento un buon modello?

Dipende davvero da alcune cose:

• Il chiamante è responsabile di sapere quale dovrebbe essere il valore?
• Ci sono più setter chiamati nel metodo?
• Qual è lo scopo ultimo del metodo chiamato?

In breve, stiamo cercando di bilanciare la leggibilità con la praticità. So che il codice di esempio usa nomi generici apposta, ma ho visto esempi reali in cui un metodo è stato chiamato qualcosa di generico come fill() . Avrei un vero problema.

Il tuo metodo interno dovrebbe essere nominato abbastanza descrittivo da poter indovinare cosa sta succedendo. Come punto intermedio, se un metodo che sto chiamando fa più cose su un'istanza, potrei sostenere che dovresti passare quell'istanza al metodo in questo modo:

private void fill(Beta beta) {
    // do all kinds of stuff
    beta.setMyField(Integer.valueOf(42));
}

Questo almeno mi induce a pensare che il Beta che ho passato cambierà. Preferirei avere questo indizio piuttosto che il metodo fill() che modifica lo stato della classe senza alcun indizio su quale stato venga modificato.

Nel tuo esempio, non esiste un vero motivo per avere un campo di classe Beta inst . Viene creato nuovo in doSomething() e passato dietro le quinte a fill() . Se apporti la modifica a fill(Beta) , puoi facilmente rimuovere il campo della classe. La tua nuova classe ha questo aspetto:

class Alpha {
    void doSomething() {
        Beta beta = new Beta();
        // Do stuff
        fill(beta);
        // Do more stuff;
        beta.getMyField();
    }

    void fill(Beta beta) {
        // do stuff
        beta.setMyField(Integer.valueOf(42));
    }
}

La differenza chiave qui è che Beta non è più un campo di classe. Ciò aumenta la probabilità che doSomething() rientri, cosa che sicuramente non era prima.

In breve:

• Passa esplicitamente le istanze, con un campo classe che causa implicitamente problemi di concorrenza e altri bug sottili
• Identifica i metodi in modo appropriato per poter avere una buona idea su ciò che stanno facendo veramente

Penso che la risposta alla tua domanda sia: dipende .

Prima di tutto guardando questo esempio il tuo problema è legato a troppe responsabilità di doThinkg() , ecco perché non è chiaro cosa fa e cosa viene modificato.

Possibile soluzione

1) Cerca di introdurre una singola responsabilità nei tuoi metodi
2) Fornire metodo privato / funzione util (Servizio con metodo statico o funzione utils se il proprio  la lingua è in grado di):

Esempio di helper statici utili:

class Alpha {
  Beta inst;
  public void initilize() {
      this.inst = AlphaHelper.prepareBeta();
  }
  public Integer getBetaField() {
    return this.inst.getMyField()
  }

}

class AlphaHelper {
  public static Beta prepareBeta() {
    Beta beta = AlphaHelper.getBetaInstance();
    AlphaHelper.initilizeBeta(beta);
    return beta;
  }

  private static getBetaInstance() {
    return new Beta();
  }

  private static initilizeBeta(Beta beta) {
    beta.setMyField(Integer.valueOf(42))
  }
}

Esempio di metodo privato

class Alpha {
  Beta inst;
  public void initilize() {
      this.inst = new Beta();
      // call other private methods or do something
      this.fillBetaInstance(this.inst);
  }
  public Integer getBetaField() {
    return this.inst.getMyField();
  }

  private void fillBetaInstance(Beta beta) {
    beta.setMyField(Integer.valueOf(42));
  }

}

Questo snippet di codice e il suo utilizzo omettono alcuni dettagli importanti. Quindi, la risposta dipende.

Un modo significativo di vederlo è lifetime of state : in particolare qui, il valore assegnato al campo (e l'uso del campo) vs la vita di tutto l'oggetto stesso.

Se il campo è un modo per passare lo stato temporaneo da doThing a fill , probabilmente è nel posto sbagliato. Qui, non possiamo sapere con certezza a causa della semplicità dell'esempio (anche se forzato).

Oppure il campo acquisisce lo stato di funzionamento prolungato inizializzato nel costruttore e ha un valore valido per tutta la durata dell'oggetto (anche se può cambiare)?

A questo punto, vorrei iniziare a guardare un'altra prospettiva importante, che è l'astrazione fornita a un programmatore client che consuma (forse voi), che, in uso , non dovrebbe riguardare l'implementazione ma piuttosto l'interfaccia, che speriamo sia semplice da usare e difficile da sbagliare.

Potrei chiedere: è fill essenzialmente privato ma non contrassegnato come tale a causa dell'esempio incompleto? In caso contrario, cosa si aspettano i chiamanti quando usano fill (forse di cosa hanno bisogno)?

È possibile che il campo inst sia nullo (non inizializzato)? Qual è il valore atteso di inst dopo la costruzione?

La costruzione non è solo per l'inizializzazione dei campi non elaborata, ma anche per legare insieme gli oggetti in un'unica astrazione. Per molti scopi, la costruzione può legare insieme gli oggetti in un'unica astrazione che dura l'intera vita dell'entità costruita. Ogni volta che il programmatore client può consumare un'astrazione, questo è un miglioramento su di loro che si destreggia insieme a più oggetti contemporaneamente, ad es. come una coppia (o più) di oggetti.

Questa è una delle ragioni per cui l'approccio immutabile che hai menzionato funziona bene. Tuttavia, possiamo avere uno stato mutevole nei nostri oggetti mentre si segue ancora un approccio che usa la costruzione per legare diversi oggetti in una singola astrazione per il client da usare (e preferibilmente forse che almeno i legami dalla costruzione sono immutabili). Questo stile suggerisce che cambiare i binding significa creare un altro oggetto - è un'istanza diversa della stessa astrazione, piuttosto che un'astrazione i cui bind sono mutevoli.

L'impostazione dei campi senza passaggio di argomenti non è male da sola. I numeri magici come 42 sono quasi sempre una cattiva idea.

Il tuo problema è inerente al fatto che le istanze di TaskProcessCases hanno uno stato mutabile, quindi ogni chiamata al metodo potrebbe potenzialmente cambiare lo stato, non ha nulla a che fare con il passare o meno gli argomenti al metodo.

Il problema è solitamente mitigato da una buona denominazione. retrieveData non mi piace come il nome di qualcosa che sta cambiando stato ma nel tuo dominio potrebbe essere diverso.

Potresti utilizzare le eccezioni personalizzate per garantire alcuni scenari obbligatori e renderlo esplicito nel tuo codice:

void doThing() throws NotConfiguredException {

    if (inst != null && inst.x > 0) { //or some other validations

        //...do your stuf here...
        //it's very explicit what are the conditions
        //required for this logic to work

    } else {
        throw new NotConfiguredException("Beta.x <= 0");
    }

}