Working with legacy code

Cerco sempre di stare attento a come scrivo il codice e prediligo sempre la leggibilità e il design rispetto ad altre metriche e ritengo che una delle prime regole sia riuscire a far capire l'utente cosa fa un metodo semplicemente leggendo la firma.
Oggi ho perso 2 ore su un'applicazione legacy per risolvere uno stupidissimo bug.
Dopo aver scavato un po' con il debugger sono riuscito ad identificare un blocco di istruzioni che causavano l'errore, tra le istruzioni chiamate c'era questa innocua istruzione:
 
collection.Add(currentKey, newValue);
 
Il nome del metodo parla chiaro e anche le variabili usate sono esplicative, quindi per i primi 119 minuti ho sempre premuto F10 quando il debugger si fermava su questa istruzione pensando che il problema fosse altrove.
Poi quando le cartucce rimaste erano poche ho provato ad entrare in quel metodo Add e vi ho trovato questo codice:
 
public void Add(String key, MyObject value)
{
if (value == null)
{
Remove(key);
}
else
{
if (ContainsKey(key))
{
this[key] = value;
}
else
{
_base.Add(key, value);
}
}
}
 
Il grosso problema è quel Remove che sta dentro un'istruzione Add!
Queste situazioni, che sono paradossali, creano grossi problemi, perchè mai un metodo Add dovrebbe rimuovere un elemento da una collezione...se si chiama Add ci sarà un perchè? O no? Se volevo che rimuovesse qualcosa l'avrei chiamato AddButRemoveIfTheValueIsNull.
Il fatto che è molto, molto importante che il nome del metodo sia sincronizzato con il suo comportamento. Mi sembra una regola semplice da applicare.

Print | posted on Wednesday, February 11, 2009 11:36 PM