Questa è una situazione corretta per usare una costante?

42

Quindi il mio professore stava restituendo un feedback su un progetto a cui ho lavorato. Ha inserito alcuni segni per questo codice:

if (comboVendor.SelectedIndex == 0) {
  createVendor cv = new createVendor();
  cv.ShowDialog();
  loadVendors();
}

Questo è in un gestore "indice modificato" della casella combinata. Viene utilizzato quando l'utente desidera creare un nuovo fornitore, la mia opzione principale (indice 0, che non cambia mai) apre la finestra di dialogo "Crea un nuovo fornitore". Quindi il contenuto della mia casella combinata finisce così:

Create New Vendor...
Existing Vendor
Existing Vendor 2
Existing Vendor 3

Il suo problema è con il primo codice di riga:

if (comboVendor.SelectedIndex == 0)

Afferma che lo 0 dovrebbe essere una costante, e in realtà mi ha bloccato per questo. Sostiene che non dovrei usare letterali nel mio codice.

Il fatto è che non capisco perché vorrei rendere quel codice in quella situazione una costante. Quell'indice non cambierà mai, né è qualcosa che dovresti modificare. Sembra uno spreco di memoria tenere un singolo 0 in memoria che viene utilizzato per una situazione molto specifica e non cambia mai.

    
posta Red 5 01.12.2011 - 21:56
fonte

14 risposte

90

Il modo corretto di farlo in C # è di non fare affidamento sull'ordinamento di ComboItems .

public partial class MyForm : Form
{
    private readonly object VENDOR_NEW = new object();

    public MyForm()
    {
        InitializeComponents();
        comboVendor.Items.Insert(0, VENDOR_NEW);
    }

    private void comboVendor_Format(object sender, ListControlConvertEventArgs e)
    {
        e.Value = (e.ListItem == VENDOR_NEW ? "Create New Vendor" : e.ListItem);
    }

    private void comboVendor_SelectedIndexChanged(object sender, EventArgs e)
    {
        if(comboVendor.SelectedItem == VENDOR_NEW)
        {
            //Special logic for selecting "create new vendor"
        }
        else
        {
            //Usual logic
        }
    }
}
    
risposta data 01.12.2011 - 23:53
fonte
83

L'ordine nella casella combinata potrebbe cambiare. Cosa succede se aggiungi un'altra opzione come "Crea fornitore speciale ..." prima di "Crea nuovo Vender ..."

Il vantaggio di usare una costante è se ci sono molti metodi che dipendono dall'ordine della casella combinata, devi solo cambiare la costante e non tutti i metodi se questo cambia.

L'uso di una costante è anche più leggibile di un letterale.

if (comboVendor.SelectedIndex == NewVendorIndex)

La maggior parte delle lingue compilate sostituirà la costante al momento della compilazione, quindi non ci sono penalizzazioni delle prestazioni.

    
risposta data 01.12.2011 - 22:09
fonte
36

Questa situazione che descrivi è una sentenza, personalmente non la userei se è usata solo una volta ed è già leggibile.

La vera risposta è che ha scelto questo per insegnarti una lezione.

Non dimenticare che è un professore, il suo compito è insegnarti la codifica e le migliori pratiche.

Direi che sta facendo davvero un ottimo lavoro.

Certo che potrebbe sembrare un po 'assoluto, ma sono certo che ci penserai di nuovo prima di usare numeri magici.

Inoltre ti ha messo abbastanza sotto la pelle per entrare a far parte di una community online di programmatori solo per scoprire cosa è considerata una buona pratica in questa situazione.

Tanto di cappello al tuo professore.

    
risposta data 01.12.2011 - 22:32
fonte
13

[...] my top option (index 0, that never changes) opens the "Create a new Vendor" dialog".

Il fatto che tu abbia dovuto spiegare questo dimostra perché dovresti usare una costante. Se hai introdotto una costante come NEW_VENDOR_DIALOG , il tuo codice sarebbe più intuitivo. Inoltre, i compilatori ottimizzano le costanti, quindi non ci sarà un cambiamento nelle prestazioni.

Scrivi programmi per programmatori, non compilatori. A meno che tu non stia specificatamente cercando di eseguire l'ottimizzazione micro, cosa che non sembra che tu sia.

    
risposta data 01.12.2011 - 23:15
fonte
12

He claims that the 0 should be a constant, and actually docked me marks because of that.

Sono d'accordo. L'uso di zero qui è "magico". Immagina di leggere questo codice per la prima volta. Non sai perché lo zero è speciale e il letterale non ti dice nulla sul perché lo zero è speciale. Se invece hai detto if(comboVendor.SelectedIndex == CreateNewVendorIndex) , diventa estremamente chiaro al primo lettore che cosa significa il codice.

He claims I shouldn't use literals in my code at all.

Questa è una posizione estrema; una posizione realistica sarebbe dire che l'uso di letterali è una bandiera rossa che indica che il codice potrebbe non essere chiaro come potrebbe essere. A volte è appropriato.

The thing is, I don't understand why I would want to make that code in that situation a constant. That index will never change

Il fatto che non cambierà mai è un motivo eccellente per renderlo una costante . Ecco perché le costanti sono chiamate costanti; perché non cambiano mai.

nor is it something that you would need to tweak.

Davvero? Non puoi vedere nessuna situazione in cui qualcuno potrebbe voler cambiare l'ordine delle cose in una casella combinata?

Il fatto che tu possa vedere una ragione per cui questo potrebbe cambiare in futuro è una buona ragione per non renderla una costante. Piuttosto dovrebbe essere un campo intero statico di sola lettura non costante. Una costante dovrebbe essere una quantità che è garantita per mantenere lo stesso per sempre . Pi e il numero atomico dell'oro sono buone costanti. I numeri di versione non lo sono; cambiano ogni versione. Il prezzo dell'oro è ovviamente una terribile costante; cambia ogni secondo. Rende solo costante cose che non cambiano mai, mai .

It seems like a waste of memory to keep a single 0 in memory that's used for a very specific situation and never changes.

Ora arriviamo al nocciolo della questione.

Questa è forse la riga più importante nella tua domanda perché indica che hai una profonda conoscenza imperfetta della (1) memoria e (2) ottimizzazione. Sei a scuola per imparare, e ora sarebbe un grande momento per avere una corretta comprensione dei fondamenti. Puoi spiegare in dettaglio perché ritieni che "sia uno spreco di memoria mantenere un solo zero in memoria"? Innanzitutto, perché credi che l'ottimizzazione dell'utilizzo di quattro byte di memoria in un processo con almeno due miliardi di byte di memoria indirizzabile dall'utente sia rilevante? Secondo, esattamente quale risorsa utilizzi immagina che venga consumato qui ? Cosa intendi con "consumo" di memoria?

Sono interessato alle risposte a queste domande perché sono l'occasione per imparare come la tua comprensione dell'ottimizzazione e della gestione della memoria non sono corrette, e secondo perché voglio sempre sapere perché i principianti credono in cose bizzarre, quindi Posso progettare strumenti migliori per portarli ad avere credenze corrette.

    
risposta data 02.12.2011 - 18:20
fonte
6

Ha ragione. Hai ragione. Ti sbagli.

Ha ragione, concettualmente, che i numeri magici dovrebbero essere evitati. Le costanti rendono il codice più leggibile aggiungendo contesto al significato del numero. In futuro, quando qualcuno legge il tuo codice, sa perché è stato usato un particolare numero. E se hai bisogno di cambiare un valore da qualche parte lungo la linea, è molto meglio cambiarlo in un unico posto piuttosto che cercare di rintracciare ovunque venga usato un particolare numero.

Detto questo, hai ragione. In questo caso specifico, non penso davvero che una costante sia giustificata. Stai cercando il primo elemento nell'elenco, che è sempre zero. Non sarà mai 23. O -pi. Stai specificamente cercando zero. Non penso davvero che sia necessario ingombrare il codice rendendolo una costante.

Hai torto, tuttavia, nell'assumere che una costante venga trascinata in una variabile, "usando la memoria". C'è una costante per l'umano e il compilatore. Indica al compilatore di mettere quel valore in quel punto durante la compilazione, dove altrimenti avresti messo un numero letterale. E anche se avesse mantenuto la costante nella memoria, per tutte le applicazioni tranne quelle più impegnative, la perdita di efficienza non sarebbe nemmeno misurabile. Preoccuparsi dell'uso della memoria di un singolo intero cade definitivamente in "ottimizzazione prematura".

    
risposta data 01.12.2011 - 22:52
fonte
2

Avrei sostituito 0 con una costante per rendere chiaro il significato, come NewVendorIndex . Non sai mai se il tuo ordine cambierà.

    
risposta data 01.12.2011 - 22:11
fonte
1

Questa è la preferenza totale del tuo professore. In genere, si utilizza una costante solo se il letterale verrà utilizzato più volte, si desidera rendere evidente al lettore qual è lo scopo della linea, o il letterale sarà eventualmente modificato in futuro e si desidera solo cambiare in un unico posto. Tuttavia, per questo semestre, il professore è il capo, quindi lo farei da ora in poi in quella classe.

Un buon allenamento per il mondo aziendale? Molto probabilmente.

    
risposta data 01.12.2011 - 22:10
fonte
1

Per essere onesti, mentre non penso che il tuo codice sia la migliore pratica, il suo suggerimento è francamente un po 'grottesco.

Una pratica più comune per una combobox .NET è di dare alla voce "Seleziona ..." un valore vuoto, mentre gli oggetti effettivi hanno valori significativi, e poi fanno:

if (string.IsNullOrEmpty(comboVendor.SelectedValue))

anziché

if (comboVendor.SelectedIndex == 0)
    
risposta data 01.12.2011 - 23:01
fonte
1

Non ha torto per sottolineare il valore dell'uso delle costanti e non hai torto nell'usare i letterali. A meno che non abbia sottolineato che questo è lo stile di codifica previsto, non si dovrebbero perdere punti per l'utilizzo di letterali poiché non sono dannosi. Ho visto letterali usati in tutto il posto così tante volte nel codice commerciale.

Il suo punto è buono però. Può essere a suo modo per renderti consapevole dei vantaggi delle costanti:

1-Proteggono il tuo codice in una certa misura da manomissioni accidentali

2-Come dice @DeadMG nella sua risposta, se lo stesso valore letterale viene usato in molti punti, può apparire con un valore diverso per errore - Quindi le costanti preservano la coerenza.

Le costanti 3 preservano il tipo, quindi non devi usare qualcosa come 0F per significare zero.

4-Per facilità di lettura, COBOL usa ZERO come parola riservata per il valore zero (ma consente anche di usare lo zero letterale) - Quindi, dare un valore a volte è utile, ad esempio: (Fonte: costanti ms

class CalendarCalc
{
    const int months = 12;
    const int weeks = 52; //This is not the best way to initialize weeks see comment
    const int days = 365;

    const double daysPerWeek = (double) days / (double) weeks;
    const double daysPerMonth = (double) days / (double) months;
}

o come nel tuo caso (come mostrato nella risposta di @Michael Krussel)

    
risposta data 01.12.2011 - 22:23
fonte
0

Dovresti solo metterlo in una costante se ha una derivazione complessa o se è ripetuto spesso. Altrimenti, un valore letterale va bene. Mettere tutto in una costante è un overkill totale.

    
risposta data 01.12.2011 - 22:02
fonte
0

In realtà, come accennato, cosa succede se la posizione cambia? Quello che potresti / dovresti fare è usare un codice, piuttosto che fare affidamento sull'indice.

Quindi, quando crei la tua lista di selezione, finisce con html come

<select>
    <option value='CREATE'>Create New Vendor...</option>
    <option value='1'>Existing Vendor</option>
    <option value='2'>Existing Vendor 2</option>
    <option value='3'>Existing Vendor 3</option>
</select>

Quindi, invece di controllare selectedIndex === 0 , controlla che il valore sia CREATECODE che sarebbe in una costante e che sarebbe stato utilizzato sia per questo test sia per la creazione dell'elenco di selezione.

    
risposta data 01.12.2011 - 23:04
fonte
0

Me ne libererei del tutto. Basta mettere un nuovo pulsante di creazione accanto all'elenco della casella combinata. Fare doppio clic su un elemento nell'elenco da modificare o fare clic sul pulsante. Non avere la nuova funzionalità sepolta nella casella combinata. Quindi il numero magico viene rimosso del tutto.

In generale qualsiasi numero letterale nel codice dovrebbe essere definito come costante in modo da mettere il contesto attorno al numero. Cosa significa zero? In questo caso 0 = NEW_VENDOR. In altri casi, potrebbe significare qualcosa di diverso, quindi è sempre una buona idea per la leggibilità e la manutenibilità di mettere un contesto intorno ad esso.

    
risposta data 01.12.2011 - 23:05
fonte
0

Come altri hanno già detto, è necessario utilizzare un metodo diverso dal numero di indice per identificare l'elemento della casella combinata che corrisponde a una determinata azione; oppure, puoi trovare l'indice con qualche logica programmatica e memorizzarlo in una variabile.

Il motivo per cui sto scrivendo è di indirizzare il tuo commento sull '"uso della memoria". In C #, come nella maggior parte delle lingue, le costanti sono "piegate" dal compilatore. Ad esempio, compilare il seguente programma ed esaminare l'IL. Scoprirai che tutti quei numeri non arrivano nemmeno nell'IL, per non parlare della memoria del computer:

public class Program
{
    public static int Main()
    {
        const int a = 1000;
        const int b = a + a;
        const int c = b + 42;
        const int d = 7928345;
        return (a + b + c + d) / (-a - b - c - d);
    }
}

IL risultante:

.method public hidebysig static 
    int32 Main () cil managed 
{
    .maxstack 1
    .locals init (
        [0] int32 CS$1$0000
    )

    IL_0000: nop
    IL_0001: ldc.i4.m1  // the constant value -1 to be returned.
    IL_0002: stloc.0
    IL_0003: br.s IL_0005

    IL_0005: ldloc.0
    IL_0006: ret
}

Quindi, se usi una costante, un valore letterale o un kilobyte di codice usando l'aritmetica costante, il valore viene trattato letteralmente nell'IL.

Un punto correlato: il piegamento costante si applica ai valori letterali stringa. Molti credono che una chiamata come questa causa troppa inutile, inefficiente concatenazione di stringhe:

public class Program
{
    public static int Main()
    {
        const string a = "a";
        const string b = a + a;
        const string c = "C";
        const string d = "Dee";
        return (a + b + c + d).Length;
    }
}

Ma controlla l'IL:

IL_0000: nop
IL_0001: ldstr "aaaCDee"
IL_0006: callvirt instance int32 [mscorlib]System.String::get_Length()
IL_000b: stloc.0
IL_000c: br.s IL_000e
IL_000e: ldloc.0
IL_000f: ret

Bottom line: gli operatori sulle espressioni costanti generano espressioni costanti e il compilatore esegue tutto il calcolo; non influisce sulle prestazioni di runtime.

    
risposta data 23.12.2011 - 20:06
fonte

Leggi altre domande sui tag