L'aggiunta di un tipo di reso a un metodo di aggiornamento viola il "Principio di responsabilità singola"?

Come per qualsiasi regola, penso che la cosa importante qui sia considerare lo scopo della regola, lo spirito, e non impantanarsi nell'analizzare esattamente come la regola è stata formulata in alcuni libri di testo e come applicarla in questo caso. Non abbiamo bisogno di avvicinarci a questo tipo di avvocati. Lo scopo delle regole è aiutarci a scrivere programmi migliori. Non è come lo scopo di scrivere programmi è quello di mantenere le regole.

Lo scopo della regola della responsabilità unica è di rendere i programmi più facili da capire e da mantenere facendo sì che ciascuna funzione faccia una cosa autonoma e coerente.

Ad esempio, una volta ho scritto una funzione che ho chiamato qualcosa come "checkOrderStatus", che determinava se un ordine era in sospeso, spedito, back-ordinato, qualunque cosa e restituiva un codice che indica quale. Poi è arrivato un altro programmatore che ha modificato questa funzione per aggiornare anche la quantità a disposizione al momento della spedizione dell'ordine. Ciò ha gravemente violato il principio della responsabilità unica. Un altro programmatore che legge quel codice in seguito vedrebbe il nome della funzione, vedere come è stato usato il valore di ritorno e potrebbe non sospettare mai che abbia fatto un aggiornamento del database. Qualcuno che aveva bisogno di ottenere lo stato dell'ordine senza aggiornare la quantità a disposizione sarebbe in una posizione scomoda: dovrebbe scrivere una nuova funzione che duplica la parte dello stato dell'ordine? Aggiungi un flag per dire se fare l'aggiornamento db? Ecc. (Ovviamente la risposta corretta sarebbe quella di interrompere la funzione in due, ma ciò potrebbe non essere pratico per molte ragioni.)

D'altra parte, non farei il pasticcio di ciò che costituisce "due cose". Recentemente ho scritto una funzione che invia informazioni sui clienti dal nostro sistema al sistema del nostro cliente. Quella funzione esegue una riformattazione dei dati per soddisfare i loro requisiti. Ad esempio, abbiamo alcuni campi che potrebbero essere nulli sul nostro database, ma non consentono null, quindi dobbiamo inserire un testo fittizio, "non specificato" o ho dimenticato le parole esatte. Probabilmente questa funzione sta facendo due cose: riformattare i dati e inviarli. Ma ho intenzionalmente messo questo in una singola funzione piuttosto che avere "riformattare" e "inviare" perché non voglio mai, mai inviare senza riformattare. Non voglio che qualcuno scriva una nuova chiamata e non si rende conto che deve chiamare reformat e quindi inviare.

Nel tuo caso, aggiorna il database e restituisci un'immagine del record scritta come due cose che potrebbero andare insieme logicamente e inevitabilmente. Non conosco i dettagli della tua applicazione, quindi non posso dire definitivamente se questa è una buona idea o no, ma sembra plausibile.

Se si sta creando un oggetto in memoria che contiene tutti i dati per il record, facendo le chiamate al database per scrivere questo e quindi restituendo l'oggetto, questo ha molto senso. Hai l'oggetto nelle tue mani. Perché non restituirlo? Se non hai restituito l'oggetto, come potrebbe ottenere il chiamante? Doveva leggere il database per ottenere l'oggetto che hai appena scritto? Sembra piuttosto inefficiente. Come avrebbe trovato il disco? Conosci la chiave primaria? Se qualcuno dichiara che è "legale" che la funzione di scrittura restituisca la chiave primaria in modo da poter rileggere il record, perché non restituire semplicemente l'intero record in modo da non doverlo? Qual è la differenza?

D'altra parte, se la creazione dell'oggetto è un po 'di lavoro abbastanza diverso dalla scrittura del record del database, e un chiamante potrebbe voler scrivere ma non creare l'oggetto, questo potrebbe essere uno spreco. Se un chiamante potrebbe volere l'oggetto ma non scrivere, allora dovresti fornire un altro modo per ottenere l'oggetto, il che potrebbe significare scrivere codice ridondante.

Ma penso che lo scenario 1 sia più probabile, quindi direi, probabilmente nessun problema.

Il concetto di SRP è quello di fermare i moduli facendo 2 cose diverse quando li fanno individualmente per una migliore manutenzione e meno spaghetti nel tempo. Poiché l'SRP dice "una ragione per cambiare".

Nel tuo caso, se avevi una routine che "aggiorna e restituisce l'oggetto aggiornato", stai ancora cambiando l'oggetto una volta, dandogli 1 motivo per cambiare. Il ritorno dell'oggetto non è né qui né là, stai ancora operando su quel singolo oggetto. Il codice ha la responsabilità di uno, e solo uno, cosa.

L'SRP non si tratta in realtà di cercare di ridurre tutte le operazioni a una singola chiamata, ma di ridurre ciò su cui si sta operando e come si sta operando su di esso. Quindi un singolo aggiornamento che restituisce l'oggetto aggiornato va bene.

Come sempre, questa è una domanda di laurea. L'SRP dovrebbe impedirti di scrivere un metodo che recuperi un record da un database esterno, esegua una trasformata di Fourier veloce e aggiorna un registro delle statistiche globale con il risultato. Penso che quasi tutti sarebbero d'accordo che queste cose dovrebbero essere fatte con metodi diversi. Postulare una responsabilità single per ogni metodo è semplicemente il modo più economico e memorabile per rendere quel punto.

All'altro estremo dello spettro ci sono metodi che forniscono informazioni sullo stato di un oggetto. Il tipico isActive ha dato questa informazione come sua unica responsabilità. Probabilmente tutti sono d'accordo sul fatto che questo sia ok.

Ora, alcuni estendono il principio fino a quel momento che considerano il ritorno di un flag di successo una responsabilità diversa dall'eseguire l'azione il cui successo viene segnalato. Sotto un'interpretazione estremamente severa, questo è vero, ma poiché l'alternativa sarebbe dover chiamare un secondo metodo per ottenere lo stato di successo, che complica il chiamante, molti programmatori stanno perfettamente bene con la restituzione dei codici di successo da un metodo con effetti collaterali. / p>

Restituire il nuovo oggetto è un ulteriore passo avanti verso molteplici responsabilità. Richiedere al chiamante di effettuare una seconda chiamata per un intero oggetto è leggermente più ragionevole che richiedere una seconda chiamata solo per vedere se il primo è riuscito o meno. Tuttavia, molti programmatori prenderebbero in considerazione la possibilità di restituire perfettamente il risultato di un aggiornamento. Mentre questo può essere interpretato come due responsabilità leggermente diverse, non è certamente uno degli abusi egregia che ha ispirato il principio per cominciare.

Violi il Principio di Responsabilità Unica?

Non necessariamente. Se qualcosa, questo viola il principio della query comando separazione.

La responsabilità è

update the employee data

con la comprensione implicita che questa responsabilità include lo stato dell'operazione; ad esempio, se l'operazione fallisce, viene generata un'eccezione, se riesce, viene restituito il dipendente dell'aggiornamento, ecc.

Ancora una volta, questa è tutta una questione di grado e giudizio soggettivo.

Che dire della separazione tra query e comandi?

Bene, quel principio esiste, ma restituire il risultato di un aggiornamento è in effetti molto comune.

(1) Java Set#add(E) aggiunge l'elemento e restituisce la sua inclusione precedente nel set.

if (visited.add(nodeToVisit)) {
    // perform operation once
}

Questo è più efficiente dell'alternativa CQS, che potrebbe dover eseguire due ricerche.

if (!visited.contains(nodeToVisit)) {
    visited.add(nodeToVisit);
    // perform operation once
}

(2) Confronta-e-scambia , fetch-and-add e test -e-set sono primitive comuni che consentono l'esistenza di una programmazione concorrente. Questi pattern vengono visualizzati spesso, dalle istruzioni della CPU di basso livello alle raccolte simultanee di livello elevato.

La singola responsabilità riguarda una classe che non ha bisogno di cambiare per più di un motivo.

Ad esempio, un dipendente ha un elenco di numeri di telefono. Quando il modo in cui gestisci i numeri di telefono cambia (potresti aggiungere codici di chiamate nazionali) non dovrebbe cambiare affatto la classe dei dipendenti.

Non vorrei che la classe dei dipendenti avesse bisogno di sapere come si salva nel database, perché cambierebbe per le modifiche nei dipendenti e le modifiche nel modo in cui i dati sono archiviati.

Simile non dovrebbe esserci un metodo CalculateSalary nella classe dei dipendenti. Una proprietà di stipendio è ok, ma il calcolo della tassa ecc. Dovrebbe essere fatto da qualche altra parte.

Ma che il metodo di aggiornamento restituisca ciò che ha appena aggiornato va bene.

Wrt. il caso specifico:

Employee Update(Employee, name, password, etc) (actually using a Builder since I have a lot of parameters).

È sembra il metodo Update prende un Employee esistente come primo parametro per identificare (?) il dipendente esistente e una serie di parametri da modificare su questo dipendente.

I do pensa che potrebbe essere fatto più pulito. Vedo due casi:

(a) Employee in realtà contiene un database / ID univoco con il quale può essere sempre identificato nel database. (Cioè, tu non hai bisogno che l'intero record sia impostato per trovarlo nel DB.

In questo caso, preferirei un metodo void Update(Employee obj) , che trova il record esistente per ID e quindi aggiorna i campi dell'oggetto passato. O forse un void Update(ID, EmployeeBuilder values)

Una variante di ciò che ho trovato utile è avere solo un metodo void Put(Employee obj) che inserisce o aggiorna, a seconda che il record (per ID) esista.

(b) Il record completo esistente è necessario per la ricerca DB, in tal caso potrebbe avere ancora più senso avere: void Update(Employee existing, Employee newData) .

Per quanto posso vedere qui, direi che la responsabilità di costruire un nuovo oggetto (o valori di sotto-oggetto) da immagazzinare e effettivamente conservarla sono ortogonali, quindi li separerei.

I requisiti concomitanti menzionati in altre risposte (atomic set-and-retrieve / compare-swap, ecc.) non sono stati un problema nel codice DB su cui ho lavorato finora. Quando si parla con un DB, penso che questo dovrebbe essere gestito normalmente a livello di transazione, non a livello di singola istruzione. (Questo non vuol dire che potrebbe non esserci un design dove am "atomic" Employee[existing data] Update(ID, Employee newData) potrebbe non avere senso, ma con DB accedervi non qualcosa che normalmente vedo.)

Finora tutti qui parlano di lezioni. Ma pensaci dal punto di vista dell'interfaccia.

Se un metodo di interfaccia dichiara un tipo di ritorno e / o una promessa implicita sul valore di ritorno, allora ogni implementazione deve restituire l'oggetto aggiornato.

Quindi puoi chiedere:

• Riesci a pensare a possibili implementazioni che non vogliono disturbare la restituzione di un nuovo oggetto?
• Riesci a pensare a componenti che dipendono dall'interfaccia (avendo un'istanza iniettata), che non ha bisogno del dipendente aggiornato come valore di ritorno?

Pensa anche agli oggetti simulati per i test unitari. Ovviamente sarà più facile deridere senza il valore di ritorno. E i componenti dipendenti sono più facili da testare, se le loro dipendenze iniettate hanno interfacce più semplici.

Sulla base di queste considerazioni puoi prendere una decisione.

E se te ne pentirai più tardi, potresti ancora introdurre una seconda interfaccia, con gli adattatori tra i due. Naturalmente tali interfacce aggiuntive aumentano la complessità della composizione, quindi è tutto un compromesso.

Il tuo approccio va bene. L'immutabilità è una strong affermazione. L'unica cosa che vorrei chiedere è: c'è un altro posto dove costruisci l'oggetto. Se il tuo oggetto non fosse immutabile, dovresti rispondere a ulteriori domande perché è stato introdotto "State". E il cambiamento di stato di un oggetto può verificarsi per diversi motivi. Quindi dovresti conoscere i tuoi casi e non dovrebbero essere ridondanti o divisi.