differenza tra input previsti e dettagli di implementazione

1

Durante una revisione del codice, ho suggerito al mio collega di avere parametri discutibili per uno dei suoi metodi. In particolare, i parametri hanno rivelato troppo sui dettagli di implementazione del metodo. Il mio collega ha sostenuto che gli input di un metodo devono rivelare i dettagli di implementazione del metodo, quindi, dove si disegna la linea? Potrei vedere un po 'cosa stava dicendo, ma ho faticato per articolare la distinzione tra "input" e "dettagli di implementazione".

Esempi contrari (sosterrebbe che tutti questi sono OK):

  • int Math.abs (int someNumber); // ok per me
  • int Math.abs (Integer someNumber); // leggermente interessante per me
  • int Math.abs (int someNumber, SignedHelper signedHelper); // SignedHelper è una classe che ti dice se i numeri sono firmati o non firmati. WTF per me.

Cose che ho detto insoddisfacenti per lui:

  • Ci si aspetterebbe che Math.abs () esegua un tipo primitivo. Perché dovrebbe essere un capitale-i Integer?
  • La presenza di SignedHelper rivela troppo come funziona Math.abs ().
  • Ci si aspetterebbe Math.abs () per capire che cos'è un numero firmato: l'utente non dovrebbe aver bisogno di preoccuparsi. Questo è un dettaglio di implementazione.
  • La presenza di SignedHelper implica che è possibile passare un'implementazione di SignedHelper che si comporta in modo diverso da quello che ti aspetteresti (che potrebbe essere una possibile caratteristica, suppongo, ma nel nostro caso wasn ' t una domanda o aspettativa a tutti). Questo è sconcertante e solleva domande sui motivi di questo metodo.
  • Se facciamo finta che Math.abs () effettivamente usi SignedHelper nella sua implementazione, è meglio testare SignedHelper separatamente che esporlo nei parametri per abilitare l'iniezione di un SignedHelper deriso. (questo era in risposta a una delle sue preoccupazioni, in cui voleva testare solo il codice che utilizzava SignedHelper, piuttosto che testare qualsiasi SignedHelper insieme al codice in questione)

Quindi, come spiegheresti la differenza tra gli input che hanno senso rispetto agli input che non hanno senso? Sento che è polemico ma allo stesso tempo mi piacerebbe essere in grado di articolare qualcosa che per me è ovvio, perché è del tutto possibile che in realtà non ho una buona comprensione.

    
posta Kevin 30.05.2018 - 10:31
fonte

3 risposte

1

Direi che "Math.abs (int someNumber, SignedHelper signedHelper)" sta violando SRP (single responsibility principal). Ecco perché sembra sbagliato.

Passi un argomento al metodo e allo stesso tempo fornisci una dipendenza di cui la classe ha bisogno.

La costruzione è una responsabilità separata per ABS e il metodo sta combinando i due. Se la classe ha bisogno di SignedHelper come dipendenza, dovrebbe essere iniettata nel costruttore.

    
risposta data 30.05.2018 - 13:06
fonte
1

La firma di un metodo è l'interfaccia con i chiamanti del metodo.

Nel tuo caso:

int Math.abs(int someNumber);

ti dice che Math.abs () prende un numero intero e restituisce un altro numero intero. Un principio importante è che un'interfaccia non dovrebbe cambiare se l'implementazione cambia.

Nel tuo contesto, se tutte le implementazioni rilevanti richiedono un oggetto SignedHelper , allora può essere OK passarlo come parametro. In questo modo hai un metodo generico Math.abs() che funziona con qualsiasi implementazione di SignedHelper .

D'altro canto, se puoi anche avere implementazioni di Math.abs() che non usano affatto SignedHelper , allora sei corretto che non ci dovrebbe essere nessun parametro SignedHelper : questo è un dettaglio di implementazione che dovrebbe rimanere nascosto all'interno dell'implementazione e non perdere l'interfaccia.

Qual è il problema nel secondo scenario? Immagina di cambiare l'implementazione con una che non usa SignedHelper . Poi

  • o hai bisogno di cambiare l'interfaccia e cambiare tutte le chiamate al metodo attraverso il tuo codice,
  • oppure mantieni l'interfaccia e ignori l'argomento nell'implementazione; i chiamanti continuano a inviare un oggetto fittizio solo perché capita di trovarsi nell'interfaccia.

Nel primo caso stai cambiando un'interfaccia: a volte è inevitabile, ma cambiare un'interfaccia è una modifica non locale, quindi dovrebbe essere evitata se possibile.

Nel secondo caso l'interfaccia non descrive il metodo correttamente. Un'API che richiede parametri fittizi non è ben progettata.

    
risposta data 29.07.2018 - 15:26
fonte
0

Queste due firme

Math.abs(int someNumber);

e

Math.abs(int someNumber, SignedHelper signedHelper);

Sono in qualche modo paragonabili alle due versioni di toString della classe Integer :

public static String toString(int i)

e

public static String toString(int i, int radix)

Entrambi i metodi calcolano un risultato da un int e possono farlo senza che sia diversamente indicato come parametro. " prendi int e fai le tue cose. "

Ma forse vuoi ottenere una rappresentazione di stringa con una diversa radice. E forse l'implementazione interna può essere variabile a tale riguardo, quindi perché non esporla come un altro parametro?

Questo è esattamente ciò che accade in entrambi i casi. Hai la possibilità di modificare il funzionamento dell'attuazione interna.

Prendi il costruttore di BufferedReader come altro esempio:

public BufferedReader(Reader in, int sz)

WTF to me.

Se non sei abituato ad avere la possibilità di specificare la dimensione di un "buffer" quando lavori con un Reader , il parametro addizionale potrebbe apparire come informazioni rivelatrici dell'implementazione interna - il che è perfettamente corretto, perché è il punto intero.

Hai la possibilità di scegliere un'implementazione specifica se necessario e in quella situazione desideri definitivamente quei parametri aggiuntivi. Se selezioni di proposito BufferedReader , è molto probabile che tu voglia specificare la dimensione del buffer.

Se selezioni di proposito Math.abs(int someNumber, SignedHelper signedHelper) , è molto probabile che tu voglia specificare quale valore SignedHelper debba essere usato.

The presence of SignedHelper implies that it's possible to pass an implementation of SignedHelper that behaves differently than what you'd expect (which could be a possible feature, I guess, but in our case wasn't an ask or expectation at all). This is puzzling and raises questions about the motives of this method.

Se la sua sconcertante o non sembra essere in qualche modo soggettiva. Trovo spesso sovraccarichi per metodi in API a cui non riesco a pensare di usarli. Ma qualcun altro potrebbe anche trovarli molto utili.

C'è qualche danno nel fornire semplicemente entrambe le versioni del metodo? Nonostante non sia un requisito, la versione che accetta SignedHelper è utilizzata in qualsiasi parte del codice?

    
risposta data 29.07.2018 - 16:19
fonte

Leggi altre domande sui tag