Sono stato leggermente sorpreso di trovare questa funzione nella nostra base di codice:
public static double ToDouble( object value )
{
if ( value is double )
{
return (double)value;
}
else if ( value is decimal )
{
return decimal.ToDouble( (decimal)value );
}
else
{
return double.Parse(
value.ToString(),
System.Globalization.CultureInfo.InvariantCulture );
}
}
Viene chiamato da diversi punti del progetto, tra cui:
- con la colonna ScannedQty di una percentuale deserializzata di% co_de,
- con le colonne Qty e ScannedQty in una
DataTable
non correlata ricevuta da una connessione ADO.NET .
Lo considero tipicamente pericoloso, pericoloso e irto di errori evasivi, mentre l'autore e il mio collega non sono d'accordo. Lo ha scritto come dispositivo universale one-for-all per leggere un doppio da un oggetto indipendentemente dall'origine e dal tipo dei dati sottostanti e lo utilizza durante la lettura di un XML in entrata e durante il caricamento dei dati dal database. A volte, ha affermato, il valore effettivo può essere un DataTable
, a volte un double
e talvolta (ad esempio quando proviene da un file .XML ) a decimal
.
Ho proposto di riscrivere l'accesso alle varie origini dati utilizzando il casting esplicito per il tipo appropriato, che è stabile per ogni caso d'uso, ad esempio:
public static double DblFromDblObj( object value )
{ return ( double )value; }
public static double DblFromDecObj( object value )
{ return ( double )( decimal )value; }
public static double DblFromStrObj( object value )
{ return Double.Parse( ( string )value, CultureInfo.InvariantCulture ); }
e sostituisci tutte le invocazioni di string
con una di queste, a seconda del tipo di dati effettivo in questione, ma è loth a farlo perché
a. sarà una perdita di tempo poiché il codice funziona così com'è, e
b. se un tipo di dati cambia nel database o nel file XML in arrivo, il programma si interromperà.
A mio parere, un protocollo rigido e non ambiguo deve essere stabilito e applicato nell'implementazione mediante funzioni di accesso pulito che falliscono in caso di mancata corrispondenza del tipo, indicando un errore nei dati e una violazione del protocollo. Questo tipo di fragilità è la base di affidabilità .
Se il programmatore non può decidere il tipo di un campo nel payload e scrive il controllo dinamico dei tipi per un'attività così banale, c'è chiaramente qualcosa di sbagliato, come la mancanza di attenzione al protocollo di scambio dati o la mera pigrizia su parte del programmatore che gli ha impedito di studiare quale tipo viene restituito dove. Ad esempio, quando si esegue una query su un database tramite ToDouble()
o un altro meccanismo basato su SQL , è assolutamente sbagliato spazzare la polvere sotto il tappeto:
object valueObj = myDataRow["ScannedQty"];
double value = Convert.ToDouble( value );
Il casting diretto del tipo è più pulito, più veloce e più trasparente (perché mostra il tipo interno non ambiguo al lettore del codice):
double value = ( double )( decimal )myDataRow["ScannedQty"]; // NOT NULL field
Pertanto, il refactoring che propongo non è una perdita di tempo ma un utile miglioramento che rende il codice più pulito e il programma più affidabile. È anche molto positivo che il programma smetterà di funzionare se il tipo sottostante dell'oggetto cambia, perché con questo token sapremo che qualcuno ha infranto il protocollo.
Questi miei argomenti, tuttavia, non sono riusciti a convincere il mio collega, quindi dovrei essere grato per le opinioni degli altri per aiutarci a raggiungere una comprensione comune. Come risolvi tali conflitti nel tuo lavoro?
P.S.: Questo sembra un forum più informale di StackOverflow , spero che non venga downvoted e chiuso come basato sull'opinione . Per favore, fammi sapere se ho torto e poi essere così bravo da suggerire un posto migliore per porre la mia domanda.
P.P.S. : Per favore, sii gentile con entrambi, perché mostrerò questa domanda al mio collega.