Come spiegare le decisioni di progettazione durante il refactoring del codice

Am I justified in making the refactoring (assuming we have time allocated for cleaning up code)?

Stai facendo altro in questo codice?

In caso contrario, questo non sembra valere la pena. È probabile che il tuo pari non abbia intenzione di utilizzare altre cose e che tu debba mantenere le cose coerenti.

Se lo sei, pulire anche questo materiale sembra un chiaro vantaggio per il minimo sforzo / rischio (anche se potresti dover aggiungere un costruttore predefinito che utilizza HttpContext per avere un impatto minimo).

How do I respond to the various refutations?

1. Sì, ma non c'è motivo di accoppiare qualcosa inutilmente quando è così facile fare le cose meglio.

2. Più codice non è necessariamente più complesso. In questo caso, è più complesso, ma banalmente così. In cambio, rende più semplice il test delle unità e il codice più flessibile.

3. Certo, sta solo spostando il problema, ma sta spostando il problema in una forma più disaccoppiata. Ti consente di affrontare le parti del problema in isolamento piuttosto che di avere un problema ancora più spiacevole.

4. "Questo è ciò che abbiamo sempre fatto" non è una scusa per perpetuare codice / design / qualcosa di brutto. Sì, dovresti mirare a mantenere il codice coerente, ma in questo caso dovresti spingere per coerenza con il modello disaccoppiato piuttosto che con l'accesso diretto HttpContext . Sì, è più lavoro, ma è la cosa giusta da fare. Se il tuo pari spinge che non c'è tempo per tutto questo ora (e ha un'alternativa che è meglio), allora forse accetti di farlo più tardi (e aggiungi un ticket / bug / storia a in realtà fallo).

5. Vedi # 4.

C'è più di un modo per scuoiare un gatto. Riprova finché non trovi una soluzione che risolva le preoccupazioni di tutti. Parte del problema è che non hai davvero infranto l'accoppiamento sul codice di cache fino in fondo. Vorrei iniziare con qualcosa di simile:

public class WidgetRepository
{
  public Widget GetWidget(int id)
  {
    return Cache.GetOrCreate<Widget>("widget_" + id, () => GetWidgetImpl(id));
  }
}

public static class Cache
{
  public static T GetOrCreate<T>(string key, Func<T> createFunc)
  {
    if (HttpContext.Current == null)
    {
      return createFunc();
    }

    T result = (T)HttpContext.Current.Cache[key];

    if (result == null)
    {
      result = createFunc();
      HttpContext.Current.Cache[key] = result;
    }           

    return result;
  }
}

Non conosco C #, quindi potrebbe esserci un modo più pulito per realizzare questa stessa idea (e per favore ignora i problemi di sintassi), ma ottieni il punto. Qui le linee di codice sono paragonabili, ma ho separato la preoccupazione per la cache in una chiamata di funzione, una che dovrebbe essere altamente riutilizzabile se questo schema si ripete tutte le volte che hai inteso. Ne consegue che YAGNI ha sempre avuto bisogno di un diverso tipo di cache, eppure fornisce un luogo separato per fare quel cambiamento.

Se hai davvero bisogno di un repository separato senza alcuna connessione alla cache, puoi renderlo una classe genitore di WidgetRepository . Se hai bisogno di supportare più tipi di cache simultaneamente, allora puoi fare l'iniezione invece di usare una classe statica. Non c'è bisogno di fare impazzire tutta l'interfaccia finché non ne hai davvero bisogno.

Ancora una volta questo è solo un punto di partenza. Il mio punto è che dovresti essere in grado di trovare un modo per rendere tutti felici.

La tua classe è chiaramente superiore perché puoi iniettare un fornitore di cache fittizia.

Sebbene il controllo NULL HttpContext.Current fermi un'eccezione generata quando vengono testati il repository o le classi che lo utilizzano. Quindi non stai offrendo alcun vantaggio concreto.

Non tenterei di giustificare il tuo codice o rispondere alle confutazioni. Hai passato del tempo a ri-factoring, i tuoi cambiamenti sono chiaramente in sintonia con le pratiche accettate, il ri-factoring è una buona cosa. Di che lamentarsi?

Dato che hai dovuto giustificare questo e la natura delle confutazioni penso sia chiaro che in realtà non c'è tempo per il refactoring. Questo è il classico 'renderlo veloce' E 'renderlo buono' E 'non spendere più denaro' tricotomia.