Valore dell'utilizzo di metodi privati rispetto a tutti i pubblici nelle classi per software interno per facilità di test unitari [duplicato]

I've left them public for ease of unit testing

Facilità di scrivere quei test, forse. Ma tu accoppi quindi strettamente quella classe a un gruppo di test che interagiscono con i suoi meccanismi interni. Ciò si traduce in test fragili: probabilmente si romperanno non appena si apportano modifiche al codice. Questo crea un vero mal di testa di manutenzione, che spesso porta le persone semplicemente a eliminare i test in quanto diventano più problemi di quanti ne valga.

Ricorda che il test unitario non significa "testare il pezzo di codice più piccolo possibile". È un test di un'unità funzionale, cioè per un insieme di input in una parte del sistema, mi aspetto questi risultati. Quella unità potrebbe essere un metodo statico, una classe o un gruppo di classi all'interno di un assembly. Puntando solo sulle API pubbliche, si imita il comportamento del sistema e quindi i test diventano meno abbinati e più robusti.

Quindi rendi i metodi privati, prendi in giro l'intero DTO di FileContents e verifica solo l'unico vero metodo pubblico. Inizialmente lavorerà di più, ma col tempo raccoglierai i vantaggi della creazione di test utili come questo.

Normalmente mi aspetto che tu eserciti le funzioni dei membri privati tramite l'interfaccia pubblica. In questo caso scriverei diversi test per alimentare diversi contesti di file, con diversi set di dati presenti per poter esercitare quei metodi.

Non penso che i tuoi test dovrebbero essere a conoscenza di quei metodi privati. Penso che siano parte dell'implementazione e il tuo test dovrebbe riguardare il funzionamento del componente, non il modo in cui è stato implementato.

This class won't ever get used outside a small team at this office

Potresti essere sorpreso di ciò che viene riutilizzato non appena lo rendi pubblico. Un'alternativa è accettare che renderlo pubblico comporterà il riutilizzo e quindi estrarre i metodi in una classe di utilità generale. In questo modo puoi testare questi metodi separatamente (dato che li hai resi pubblici) e dato che si tratta di una classe di utilità pubblica, non stai asserendo implicitamente che verranno utilizzati solo nello scenario questo al momento concentrati su

Penso che il problema derivi dal design. Il mio istinto dice che hai scritto i test dopo il codice, o che hai già avuto una implementazione completa in mente quando hai iniziato a scrivere i test, e poi hai appena calzato i test per adattarli al design.

Penso che questo tipo di problema possa essere evitato ricordando il breve ciclo TDD di una piccola asserzione e facendola passare nel modo più semplice possibile.

Parli del fatto che è difficile esercitare tutti i metodi privati con metodi pubblici. Questo probabilmente indica che la tua classe fa troppo. Se non riesci a costruirlo aggiungendo test dopo test e vedendo solo i requisiti che sono stati soddisfatti e quindi rifattiandoli in codice manutenibile e leggibile, allora non hai abbastanza conoscenze sull'entità per implementarlo, oppure è troppo complesso ( sì sì, è una generalizzazione, so che sono cattivi).

Guardando i nomi dei metodi mi sembra che questa classe abbia troppe responsabilità. Ha diverse implementazioni di algoritmi, gestisce i thread, addirittura legge i file dal disco. Suddividi il tuo lavoro in pezzi riutilizzabili gestibili. Gli algoritmi di corrispondenza / convalida possono essere facilmente iniettati (come strategie, più preferibilmente imo, come delegati), la gestione dei thread dovrebbe probabilmente verificarsi a un livello superiore, ecc.

Quando non hai più una grande classe complicata con miliardi di responsabilità (beh, più o meno), il test diventa quasi banale.

Sono d'accordo con @BrianAgnew e @kai, ma vorrei aggiungere più di un commento.

Mentre un IDedupeFiler (o qualsiasi altra cosa) dovrebbe essere testato attraverso la sua interfaccia pubblica, l'OP ha deciso che c'è valore nel testare le singole sotto-routine. Indipendentemente dalle dimensioni del file o dal conteggio delle righe (che è solo un conteggio approssimativo delle proxy per le responsibilità di classe), l'OP ha deciso che c'è troppa complessità da testare dall'inizio di questa classe.

Questa è una buona cosa, per inciso, uno dei motivi per cui alla gente piace TDD è che la necessità di test costringe il programmatore ad adattare (e migliorare) il loro design. È importante sottolineare che i test precedenti sono scritti prima che questo processo avvenga, ma l'OP non è affatto lontano dalla strada delle decisioni progettuali non verificabili e il refactoring non sarà oneroso.

La domanda sembra essere se l'OP dovrebbe (1) rendere pubblici i metodi e ottenere testabilità con meno refactoring, o se dovrebbero fare qualcos'altro. Sono d'accordo con @kai e dico l'OP dovrebbe (2) dividere questa classe in blocchi isolati, testabili separatamente.

(1) interrompe l'incapsulamento e rende meno immediatamente evidente quale sia l'uso e l'interfaccia pubblica della classe. Penso che l'OP riconosca che questa non è la migliore scelta di progettazione OOP nella loro domanda. (2) significa più classi, ma non penso che sia un grosso problema, e fornisce testabilità senza compromessi di progettazione.

Se non sei d'accordo con me sul fatto che i tuoi sottoprogrammi rappresentano preoccupazioni separate e verificabili separatamente, allora non provare a scavare nella classe per testarli. Esercitali con il tuo metodo pubblico migliore . Quanto è difficile trovare questo sarà un buon indicatore se questa è stata la scelta giusta.

Penso che potresti beneficiare di un leggero cambiamento nel modo di vedere i test unitari. Invece di pensare a loro come un modo per garantire che tutto il tuo codice funzioni, pensa a loro come a un modo per garantire che la tua interfaccia pubblica faccia quello che tu rivendichi.

In altre parole, non ti preoccupare di testare gli interni - scrivi i test delle unità che dimostrano che quando assegni alla tua classe X, ottieni delle uscite Y - il gioco è fatto. Il modo in cui riesce a farlo non ha alcuna importanza. In effetti, i metodi privati potrebbero essere totalmente sbagliati, inutili, ridondanti, ecc., E fintanto che l'interfaccia pubblica fa quello che dovrebbe fare, dal punto di vista del test unitario, sta funzionando.

Questo è importante perché vuoi essere in grado di tornare indietro e rifattorare quel codice più tardi quando esce una nuova libreria che lo fa meglio, o ti rendi conto che stavi facendo del lavoro non necessario, o semplicemente decidi che il modo in cui vanno le cose nominato e organizzato potrebbe essere reso più chiaro. Se i tuoi test riguardano solo l'interfaccia pubblica, sei libero di farlo senza preoccuparti di rompere le cose.

Se fai questo cambiamento nel modo in cui pensi ai test unitari e scopri che è ancora difficile scrivere il test per una determinata classe, allora è un buon segno che il tuo design ha bisogno di alcuni miglioramenti. Forse la classe dovrebbe provare a fare di meno - forse alcuni di quei metodi privati appartengono davvero a una nuova classe più piccola. O forse hai bisogno di usare l'integrazione delle dipendenze per gestire le dipendenze esterne.

In questo caso, senza conoscere ulteriori dettagli su come si sta facendo questa deduplicazione, direi che i metodi che si desidera rendere pubblici potrebbero effettivamente essere migliori come classi separate o come metodi pubblici in una libreria di utilità. In questo modo, è possibile definire l'interfaccia per ciascuno insieme al relativo intervallo di input consentiti e output previsti e verificarli separatamente dallo script di deduplicazione stesso.

Oltre alle altre risposte, c'è un altro vantaggio nel rendere private queste funzioni e testarle tramite l'interfaccia pubblica.

Se raccogli metriche di copertura del codice sul tuo codice, diventa più facile capire quando una funzione non viene più utilizzata. Ma, se rendi pubbliche tutte queste funzioni, e fai dei test unitari per loro, allora avranno sempre almeno il test unitario che li chiama anche quando nient'altro.

Considera questo esempio:

public void foo() {
    bar();
    baz();
}

public void bar() { ... }

public void baz() { ... }

Quindi esegui test di unità individuali per foo , bar e baz . Quindi saranno tutti chiamati dal tuo framework di test unitario e avranno tutti la copertura del codice che dice che vengono utilizzati.

Considera ora questa modifica:

public void foo() {
    bar();
}

public void bar() { ... }

public void baz() { ... }

Dato che i tuoi test di unità continuano a chiamare tutte queste funzioni, la copertura del tuo codice sta per dire che vengono tutti utilizzati. Ma baz non viene più chiamato da nessun'altra parte del tuo software oltre ai test unitari. È effettivamente un codice morto.

L'avevi scritto in questo modo:

public void foo() {
    bar();
}

private void bar() { ... }

private void baz() { ... }

Quindi si perderebbe il 100% della copertura del codice di baz quando foo è cambiato, e questo è il segnale per rimuoverlo dal codice. OPPURE, ciò solleva una bandiera rossa perché baz è in realtà un'operazione super-importante, e la sua omissione causa altri problemi (si spera che, se questo è il caso, allora altri test di unità potrebbero prenderlo, ma forse no).

Vorrei separare la funzione e i casi di test per separare le classi. Quello contiene la funzione, l'altro contiene i casi di test che chiama la funzione e asserisce se il risultato è uguale a quello che ti aspetti. Non sono in C ma in JAVA userei Junit per questo. Questo divide la logica e rende la tua lezione più leggibile. Inoltre, non devi preoccuparti del tuo problema.

Non è necessario dichiarare i metodi pubblici solo per testarli unitariamente.

Il test unitario normalmente dovrebbe appartenere allo stesso pacchetto in cui appartiene la classe in prova (ovviamente all'interno della cartella del codice sorgente diversa). Di conseguenza, per il test è possibile accedere anche ai metodi privati e protetti. Ad esempio layout Maven .

Sebbene non valga la pena scrivere test molto dettagliati per tutti i componenti interni, l'ambito di visibilità non è l'unico indicatore per identificare l'unità.