Quale codice odore descrive meglio questo codice? [chiuso]

Question

Quale codice odore descrive meglio questo codice? [chiuso]

#1 da (2 voti)
#2 da (1 voti)
#3 da (0 voti)

0

Supponiamo di avere questo codice in una classe:

private DataContext _context;

public Customer[] GetCustomers() 
{
    GetContext();

    return _context.Customers.ToArray();
}

public Order[] GetOrders() 
{
    GetContext();

    return _context.Customers.ToArray();
}

// For the sake of this example, a new DataContext is *required* 
// for every public method call
private void GetContext()
{
    if (_context != null) 
    {
        _context.Dispose();
    }

    _context = new DataContext();
}

Questo codice non è thread-safe - se due chiamate a GetOrders / GetCustomers sono fatte contemporaneamente da thread diversi, possono finire usando lo stesso contesto, o il contesto potrebbe essere eliminato mentre è Usato. Anche se questo bug non esisteva, tuttavia, continua a "odorare" come un cattivo codice.

Un design molto migliore sarebbe per GetContext per restituire sempre una nuova istanza di DataContext e per sbarazzarsi del campo privato e per disporre dell'istanza una volta terminata. Passare da un campo privato inappropriato a una variabile locale sembra una soluzione migliore.

Ho esaminato gli elenchi degli odori del codice e non riesco a trovarne uno che lo descriva. In passato l'ho pensato come accoppiamento temporale , ma la descrizione di Wikipedia suggerisce che non è il termine:

Temporal coupling
When two actions are bundled together into one module just because they happen to occur at the same time.

Questa pagina discute l'accoppiamento temporale , ma l'esempio è l'API pubblica di una classe, mentre la mia domanda riguarda il design interno.

Questo odore ha un nome? O è semplicemente "codice bacato"?

code-quality coupling code-smell

posta Paul Stovell 13.03.2012 - 09:57

fonte

3 risposte

Leggi altre domande sui tag code-quality coupling code-smell

Differenze tra processi e tecniche di test del software? In quali altri luoghi oltre a flussi infiniti e liste infinite viene memorizzata la lazyness utile?

score 2 · Answer 1

Non conosco un nome accattivante per questo, ma direi che quel codice "si basa su effetti collaterali "- GetContext() non è in realtà ottenere nulla, sta cambiando qualcosa di là che influisce sul modo in cui il codice successivo verrà eseguito.

Detto questo, questo odore di codice può evolvere in un antipattern chiamato accoppiamento sequenziale . La differenza è che l'antipattern si applica di solito alle chiamate dall'esterno dell'oggetto, piuttosto che all'interno.

L'accoppiamento sequenziale di solito si manifesta più in questo modo:

obj = Cls()
obj.Start()
obj.LoadParams()
obj.InitEviron()
obj.Run()

dove eseguirli in un ordine diverso (o ometterne uno) produrrebbe risultati imprevisti.

score 1 · Answer 2

Se davvero deve istanziare il contesto in ogni chiamata (non so perché dovresti farlo), rendilo esplicito

public Customer[] GetCustomers() 
{
    using (var context = new DBContext())
    {
         return context.Customers.ToArray();
    }
}

public Order[] GetOrders() 
{
    using (var context = new DBContext())
    {
        return context.Customers.ToArray();
    }
}

Questo non è ancora il codice perfetto in quanto è strettamente legato alla classe DBContext e impossibile da isolare per il test dell'unità

score 0 · Answer 3

0

Il suo codice bacato.

    if (_context == null) 
    {
        _context.Dispose();
    }

Se mai _context == null, chiamerai un metodo su un oggetto nullo, quindi cadrà. Non c'è motivo di ripristinare il datacontext per ogni chiamata.

risposta data 13.03.2012 - 10:14

fonte