Conversione di classi di utilità statiche in singleton

3

Nell'azienda in cui lavoro, abbiamo molte classi di "utilità", ognuna con un sacco di codice all'interno (migliaia di righe) e sono tutte statiche. E un metodo statico chiama anothers. Il problema qui è che quando si vogliono testare altre classi si ha un sacco di dolore quando la funzione testata usa queste classi di utilità statiche all'interno perché non è possibile sovrascriverle (poiché sono statiche). Usiamo il mockito per i test. Powermock non è un'opzione perché a volte rompe bytecode ed è molto più lento di mockito.

Ciò che abbiamo deciso - è di trasferirli tutti in singleton, passo dopo passo, così durante il test possiamo prendere in giro l'istanza di singleton e il metodo di test che vogliamo (o anche testare qualche metodo di utilità perché devono anche essere coperti dagli autotest). È semplice farlo per le classi piccole, ma nel nostro caso ci sono 6006 linee di classe, 1261 linee di classe, 2231 linee di classe e così via. Convertirli tutti in singleton manualmente è molto doloroso, quindi c'è un modo per farlo automaticamente? O forse almeno qualche aiutante.

Esempio di codice non testabile:

static Object methodA()
{
      *very heavy code here*
}

static void methodB()
{
      *some code*
      xx = methodA();
      *here is code I've changed and want to test*
      *some code*
}

Immaginiamo di aver cambiato qualcosa in methodB e di voler testare la mia modifica. Quindi non voglio che questo metodo venga chiamato in quanto non correlato al test. Ma non posso cambiare il metodo con Mockito in questo caso. Tuttavia è possibile se all'interno del metodoA c'è una chiamata allo stesso metodo Singleton che può essere deriso perché non è statico.

Quindi se lo converti in qualcosa di simile

public class A {
    private A(){}

    public Object methodAInner()
    {
        *heavy code here*
    }
    public void methodBInner()
    {
        *some code*
        xx = methodA();
        *here is code I've changed and want to test*
            *some code*
    }


    private static A getInstance()
    {
        return SpringUtils.getBean(A.class); // here can be anything that supports testing
    }

    static Object methodA()
    {
        return getInstance().methodAInner();
    }

    static void methodB()
    {
        getInstance().methodBInner();
    }
}

Puoi fare in modo che questi SpringUtil restituiscano l'oggetto mocked con il metodo reale B e non apportino quasi nessuna modifica al codice esistente (utile per git blame), rimuovi solo i modificatori statici ei nomi dei metodi di chnge. Una cosa è la più importante per l'eredità: consente di non modificare il codice che utilizza questa classe di utilità.

    
posta maxpovver 20.08.2015 - 12:35
fonte

2 risposte

5

Sono tutto per combattere il fuoco con il fuoco, ma trasformare un brutto codice (un sacco di metodi statici) in un altro codice errato (un sacco di singleton) non sembra una buona idea trascorrere del tempo davvero.

Da ciò che hai scritto, questo è tutto in un contesto di codice legacy, ma hai cambiato accesso alla fonte in cui risiedono quei metodi statici (forse tutti, ma il resto è troppo grande per essere toccato). Diamo un'occhiata a dove vuoi andare alla fine:

  • Perché vuoi fare quel lavoro a tutti? perché il codice corrente non è testabile e rende difficile testare anche il nuovo codice
  • Perché è impossibile da testare? a causa di tutti i metodi statici
  • Perché abbiamo quei metodi statici? perché sono usati ovunque, hanno chiamate ricorsive (fanno affidamento l'uno sull'altro) e quindi sono difficili da eliminare
  • Perché si fidano l'uno dell'altro? perché le classi di utilità tendono a fare molte cose invece di una singola responsabilità

La mia prossima domanda e il tuo suggerimento saranno: Perché non estrai una responsabilità dopo l'altra?

Potresti avere difficoltà a identificare una responsabilità, ma probabilmente hai già un caso concreto di utilizzo che ti ha reso consapevole di questo problema, quindi dovrebbe essere un buon inizio.

Una volta disponibile una singola classe di responsabilità e testabile separata, puoi ancora usarla all'interno della vecchia classe per sostituire parti di quella classe. Forse hai ancora qualche duplicazione, perché hai avuto bisogno del codice di altri metodi statici nella tua nuova classe, ma è ancora chiamato da qualche altro? Quindi è un puntatore alla tua prossima area di lavoro: identifica la cosa comune che i due luoghi condividono e estrae da entrambi.

A causa della vastità delle dimensioni che ritraggono, questo sarà un processo lento e di lunga durata, quindi ogni piccola responsabilità che è fuori da quella classe deve essere considerata una vittoria. Consenti al vecchio metodo statico di fare affidamento sulla nuova classe, deprecarla e sradicarla ogni volta che vedi un sito di chiamata. Queste cose richiedono tempo prima che tu arrivi a un punto, dove puoi finalmente fare l'ultimo investimento e farti bucare la tua squadra per un giorno o due per sbarazzarti di tutto il restante codice vecchio e problematico. Finché ti preoccupi, nessuno usa questi vecchi metodi per qualsiasi codice nuovo o modificato, ti stai avvicinando a questo obiettivo. Ti auguro buona fortuna per il viaggio.

Addendum - basato sulle informazioni aggiuntive che queste classi di utilità differiscono tra diversi rami e gruppi diversi lavorano con ciascuno di questi: In breve, si sta tentando di sparare a un obiettivo in esecuzione. Data la dimensione della base di codice, l'attività stessa è abbastanza difficile per un obiettivo stabile, ma non posso raccomandarti di continuare su quel percorso, finché non hai prima fermato quel movimento. Parla con le altre squadre e falli salire a bordo. Se si oppongono in qualsiasi modo forma o forma, un compito già difficile diventa praticamente impossibile. Spero che tutti condividano il tuo dolore (o almeno non ti preoccupi di quella parte della base di codice) e puoi tornare a uno stato unificato di queste classi unendole tutte.

Sì, suggerisco di unire tutti questi rami w.r.t. le classi di utilità. So che è il suo inferno. Hai solo tre scelte: rendilo un problema comune (e condividi la soluzione!), Interrompi completamente te stesso o lascia che sia così com'è.

Il taglio significa che la tua squadra sul tuo ramo si definisce come una linea di prodotti diversa. Smetti di unirti agli altri ragazzi e tira il tuo. Potrebbe non funzionare, e anche se lo fosse, ho visto questo tornare a colpire tutti in azienda da qualche anno lungo la strada.

Comunque, ogni problema che non può essere risolto localmente all'interno della propria squadra (il codebase) è purtroppo immediatamente una questione di politica aziendale. È facile scegliere un combattimento con i tuoi compagni di squadra per sbarazzarti di una brutta classe nel codice della tua squadra, ma vale la pena pensarci due volte se vuoi davvero andare in guerra coinvolgendo gli altri team / stakeholder / manager / ecc.

    
risposta data 20.08.2015 - 14:58
fonte
2

Trovo estremamente raro che sia necessario prendere in giro i metodi di una classe di utilità. Un principio OOP molto importante è che lo stato è rappresentato dagli oggetti e che stabilisce che i metodi statici, che non operano su un oggetto, dovrebbero essere apolidi.

Non hai bisogno di prendere in giro la funzionalità . Se si sta testando il codice che dipende dall'implementazione fattoriale, non è necessario prendere in giro la funzione fattoriale. Non perché ha un'implementazione semplice, ma perché si paga molto e si guadagna molto poco dal deriderlo. L'unica cosa che ottieni è la possibilità di testare quel codice anche quando factorial è rotto, il che non vale la pena di tutto il disordine necessario per introdurre il codice e tutti i principi di programmazione che devi rompere per rendere possibile questo scherzo. Se devi assicurarti che factorial(6) restituisca 720 , anziché (la sintassi potrebbe non essere accurata):

Factorial factorial = Mockito.mock(Factorial.class);
test.when(factorial.factorial(6)).thenReturn(720);

Non sarà meglio aggiungere a Factorial s unittests:

Assert.assertEquals(Factorial.factorial(6), 720);

Ora puoi essere certo che factorial(6) restituirà 720 e non è necessario prenderlo in giro.

Quindi, non prendi in giro le funzionalità. Ciò che vuoi deridere è risorse . Si prende in giro un servizio Web in modo che il test non effettui chiamate effettive al servizio web. Si prende in giro un database in modo che i test non modifichino i dati effettivi nel database. Piuttosto che rendere i test più complessi (come il mocking del factorial), prendere in giro queste cose rende i test più semplici, perché ti fa rivivere la necessità di distribuire un servizio web o un database dall'interno del test dell'unità. Ne vale la pena.

Quindi, devi solo prendere in giro le risorse e le risorse si traducono naturalmente in oggetti! Non si chiama una funzione in un servizio Web utilizzando un metodo statico: si dispone di un oggetto che rappresenta il servizio Web (e contiene dati come il suo URL). Per la natura stessa delle risorse dovresti passarle in giro anche se non intendi scrivere test unitari, il che significa che non hai bisogno di rovinare il tuo codice per deriderlo.

Quindi, se questi metodi di utilità non rappresentano azioni sulle risorse, non è necessario prenderli in giro. E se lo fanno - in primo luogo non dovrebbero essere stati metodi di utilità!

    
risposta data 20.08.2015 - 15:24
fonte

Leggi altre domande sui tag