Czy to poprawna sytuacja, aby użyć stałej?

Więc mój profesor przekazał informacje zwrotne na temat projektu, nad którym pracowałem. Zadokował kilka znaków dla tego kodu:

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

Jest to w module obsługi „zmiany indeksu” combobox. Jest używany, gdy użytkownik chce utworzyć nowego dostawcę, moja najwyższa opcja (indeks 0, który nigdy się nie zmienia) otwiera okno dialogowe „Utwórz nowego dostawcę”. Tak więc zawartość mojego pola kombi wygląda następująco:

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

Jego problem dotyczy kodu pierwszego wiersza:

if (comboVendor.SelectedIndex == 0)

Twierdzi, że 0 powinno być stałe i dlatego właśnie zadokował mi znaki. Twierdzi, że w ogóle nie powinienem używać literałów w moim kodzie.

Chodzi o to, że nie rozumiem, dlaczego chciałbym, aby ten kod był stały. Ten indeks nigdy się nie zmieni, ani nie jest to coś, co trzeba by poprawić. Wydaje się, że marnowaniem pamięci jest utrzymanie pojedynczego 0 w pamięci, która jest używana w bardzo konkretnej sytuacji i nigdy się nie zmienia.

Właściwy sposób na zrobienie tego w języku C # nie polega wcale na kolejności elementów 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
        }
    }
}

Kolejność w polu kombi może ulec zmianie. Co jeśli dodasz inną opcję, np. „Utwórz dostawcę specjalnego ...” przed „Utwórz nowego sprzedawcę ...”

Zaletą używania stałej jest to, że jeśli istnieje wiele metod, które zależą od kolejności pola kombi, wystarczy zmienić stałą, a nie wszystkie metody, jeśli to się zmieni.

Używanie stałej jest również bardziej czytelne niż literał.

if (comboVendor.SelectedIndex == NewVendorIndex)

Większość skompilowanych języków zastąpi stałą w czasie kompilacji, więc nie ma ograniczenia wydajności.

Opisana przez ciebie sytuacja to wywołanie osądu, osobiście nie użyłbym jednego, gdyby był użyty tylko raz i jest już czytelny.

Prawdziwa odpowiedź brzmi jednak, że wybrał to, aby dać ci nauczkę.

Nie zapominaj, że jest profesorem, jego zadaniem jest nauczyć cię kodowania i najlepszych praktyk.

Powiedziałbym, że wykonuje całkiem dobrą robotę.

Pewnie, że może trochę zejść absolutnie, ale jestem pewien, że pomyślisz jeszcze raz przed użyciem magicznych liczb.

Dostał się pod twoją skórę na tyle, że możesz dołączyć do internetowej społeczności programistów, aby dowiedzieć się, co jest uważane za najlepszą praktykę w tej sytuacji.

Czapki z głów dla twojego profesora.

[...] moja najwyższa opcja (indeks 0, który nigdy się nie zmienia) otwiera okno dialogowe „Utwórz nowego dostawcę”.

Fakt, który musiałeś wyjaśnić, dowodzi, dlaczego powinieneś używać stałej. Jeśli wprowadzisz stałą NEW_VENDOR_DIALOG, kod będzie bardziej zrozumiały. Poza tym kompilatory optymalizują stałe, więc nie nastąpi zmiana wydajności.

Pisz programy dla programistów, a nie kompilatorów. Chyba że specjalnie próbujesz mikrooptymalizować, co nie wydaje się, że jesteś.

Twierdzi, że 0 powinno być stałe i dlatego właśnie zadokował mi znaki.

Zgodziłbym się. Użycie zera tutaj jest „magią”. Wyobraź sobie, że czytasz ten kod po raz pierwszy. Nie wiesz, dlaczego zero jest wyjątkowe, a dosłowność nic nie mówi o tym, dlaczego zero jest wyjątkowe. Jeśli zamiast tego powiedziałeś, if(comboVendor.SelectedIndex == CreateNewVendorIndex)to dla czytelnika po raz pierwszy staje się niezwykle jasne, co oznacza kod.

Twierdzi, że w ogóle nie powinienem używać literałów w moim kodzie.

To skrajna pozycja; realistycznym stanowiskiem byłoby stwierdzenie, że użycie literałów jest czerwoną flagą wskazującą, że kod może nie być tak jasny, jak mógłby być. Czasami jest to właściwe.

Chodzi o to, że nie rozumiem, dlaczego chciałbym, aby ten kod był stały. Ten indeks nigdy się nie zmieni

To, że nigdy się to nie zmieni, jest doskonałym powodem, aby uczynić go stałym . Właśnie dlatego stałe nazywane są stałymi; ponieważ nigdy się nie zmieniają.

nie jest to też coś, co trzeba by poprawić.

Naprawdę? Nie widzisz żadnej sytuacji, w której ktoś mógłby chcieć zmienić kolejność rzeczy w polu kombi?

Fakt, że widzisz powód, dla którego może się to zmienić w przyszłości, jest dobrym powodem, aby nie zmieniać go na stałe. Powinno to raczej być niestałe pole liczb całkowitych tylko do odczytu. Stała powinna być ilość, która gwarantuje, że pozostanie taka sama przez cały czas . Pi i liczba atomowa złota są dobrymi stałymi. Numery wersji nie są; zmieniają każdą wersję. Cena złota jest oczywiście straszną stałą; zmienia się co sekundę. Rób tylko stałe rzeczy, które nigdy, nigdy się nie zmienią .

Wydaje się, że marnowaniem pamięci jest utrzymanie pojedynczego 0 w pamięci, która jest używana w bardzo konkretnej sytuacji i nigdy się nie zmienia.

Teraz dochodzimy do sedna sprawy.

Jest to być może najważniejsza linia w twoim pytaniu, ponieważ wskazuje, że masz dogłębne zrozumienie (1) pamięci i (2) optymalizacji. Jesteś w szkole, aby się uczyć, a teraz byłby świetny czas na prawidłowe zrozumienie podstaw. Czy możesz szczegółowo wyjaśnić, dlaczego uważasz, że „marnowanie pamięci w pamięci jest marnotrawstwem”? Po pierwsze, dlaczego uważasz, że optymalizacja wykorzystania czterech bajtów pamięci w procesie z co najmniej dwoma miliardami bajtów pamięci dostępnej dla użytkownika jest istotna? Po drugie, jakie zasoby według ciebie są tutaj konsumowane ? Co masz na myśli mówiąc, że „pamięć” jest konsumowana?

Interesują mnie odpowiedzi na te pytania po pierwsze, ponieważ są one okazją, aby dowiedzieć się, w jaki sposób rozumiesz optymalizację i zarządzanie pamięcią, a po drugie, ponieważ zawsze chcę wiedzieć, dlaczego początkujący wierzą w dziwne rzeczy, dzięki czemu mogę zaprojektować lepsze narzędzia, które doprowadzą ich do prawidłowych przekonań.

On ma rację. Masz rację. Jesteś w błędzie.

Ma rację, koncepcyjnie, że należy unikać magicznych liczb. Stałe zwiększają czytelność kodu poprzez dodanie kontekstu do znaczenia liczby. W przyszłości, gdy ktoś przeczyta Twój kod, będzie wiedział, dlaczego użyto określonej liczby. A jeśli musisz zmienić wartość gdzieś poniżej linii, zdecydowanie lepiej jest zmienić ją w jednym miejscu, niż próbować szukać wszędzie tam, gdzie używana jest określona liczba.

To powiedziawszy, masz rację. W tym konkretnym przypadku naprawdę nie sądzę, aby stała była uzasadniona. Szukasz pierwszego elementu na liście, który zawsze wynosi zero. Nigdy nie będzie 23. Or -pi. W szczególności szukasz zera. Naprawdę nie sądzę, że musisz zaśmiecać kod, czyniąc go stałym.

Mylisz się jednak, zakładając, że stała jest przenoszona jako zmienna, „wykorzystując pamięć”. Istnieje stała dla człowieka i kompilatora. Mówi kompilatorowi, aby umieścił tę wartość w tym miejscu podczas kompilacji, gdzie w innym przypadku wstawiłbyś literalną liczbę. I nawet gdyby nosił stałą w pamięci, dla wszystkich oprócz najbardziej wymagających aplikacji, utrata wydajności nie byłaby nawet mierzalna. Martwienie się o wykorzystanie pamięci przez jedną liczbę całkowitą zdecydowanie wpada w „przedwczesną optymalizację”.

Chciałbym zastąpić 0stałą, aby wyjaśnić znaczenie, takie jak NewVendorIndex. Nigdy nie wiadomo, czy Twoje zamówienie ulegnie zmianie.

To jest całkowita preferencja twojego profesora. Zazwyczaj używasz stałej tylko wtedy, gdy literał zostanie użyty wiele razy, chcesz, aby czytelnik zrozumiał, jaki jest cel linii, lub twój literał prawdopodobnie zostanie zmieniony w przyszłości i chcesz tylko zmienić w jednym miejscu. Jednak w tym semestrze profesor jest szefem, więc odtąd robiłbym to w tej klasie.

Dobre szkolenie dla świata korporacji? Całkiem możliwe.

Szczerze mówiąc, chociaż nie sądzę, aby twój kod był najlepszą praktyką, jego sugestia jest szczerze mówiąc trochę groteskowa.

Bardziej powszechną praktyką dla comboboxa .NET jest nadanie pustej wartości pozycji „Wybierz ..”, podczas gdy rzeczywiste pozycje mają znaczące wartości, a następnie:

if (string.IsNullOrEmpty(comboVendor.SelectedValue))

zamiast

if (comboVendor.SelectedIndex == 0)

Nie myli się, podkreślając wartość używania stałych, a ty nie mylisz się w używaniu literałów. O ile nie podkreślił, że jest to oczekiwany styl kodowania, nie powinieneś tracić ocen za używanie literałów, ponieważ nie są one szkodliwe. Tak wiele razy widziałem dosłownie wszędzie używane w kodzie komercyjnym.

Jego punkt jest jednak dobry. Może to być jego sposób, aby uświadomić ci zalety stałych:

1-W pewnym stopniu chronią Twój kod przed przypadkowym naruszeniem

2-Jak mówi @DeadMG w swojej odpowiedzi, jeśli ta sama wartość literału jest używana w wielu miejscach, może przez pomyłkę pojawić się z inną wartością - więc stałe zachowują spójność.

3-Stałe zachowują typ, więc nie musisz używać czegoś takiego jak 0F, aby oznaczać zero.

4-Aby ułatwić czytanie, COBOL używa ZERO jako słowa zarezerwowanego dla wartości zero (ale pozwala także na użycie dosłownego zera) - Tak więc podanie wartości nazwy jest czasem pomocne, na przykład: (Źródło: ms-Constants

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;
}

lub jak w twoim przypadku (jak pokazano w odpowiedzi @Michael Krussel)

Będziesz musiał ustawić ją na stałą tylko wtedy, gdy ma złożoną pochodną lub jeśli często się powtarza. W przeciwnym razie dosłowność jest w porządku. Stawianie wszystkiego na stałe to całkowita przesada.

Właściwie, jak wspomniano, co się stanie, jeśli zmieni się pozycja? Możesz / powinieneś / powinnaś użyć kodu zamiast polegać na indeksie.

Tak więc, kiedy tworzysz listę wyboru, kończy się ona na html like

<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>

Następnie zamiast sprawdzać selectedIndex === 0, sprawdź, czy wartość jest CREATECODEstałą, która byłaby używana zarówno w tym teście, jak i podczas tworzenia listy wyboru.

Pozbyłbym się tego całkowicie. Wystarczy umieścić przycisk Utwórz nowy obok listy pól kombi. Kliknij dwukrotnie pozycję na liście, aby edytować, lub kliknij przycisk. Nie przechowuj nowej funkcjonalności w combobox. Następnie magiczna liczba jest całkowicie usuwana.

Zasadniczo każdą literalną liczbę w kodzie należy zdefiniować jako stałą, aby umieścić kontekst wokół liczby. Co oznacza zero? W tym przypadku 0 = NEW_VENDOR. W innych przypadkach może to oznaczać coś innego, więc zawsze dobrym pomysłem jest czytelność i łatwość konserwacji, aby umieścić w tym kontekście pewien kontekst.

Jak powiedzieli inni, powinieneś użyć innej metody niż numer indeksu, aby zidentyfikować element pola kombi, który odpowiada danej akcji; lub możesz znaleźć indeks z logiką programową i zapisać go w zmiennej.

Piszę dlatego, by zająć się twoim komentarzem na temat „wykorzystania pamięci”. W języku C #, jak w większości języków, stałe są „składane” przez kompilator. Na przykład skompiluj następujący program i sprawdź IL. Przekonasz się, że wszystkie te liczby nawet nie trafiają do IL, nie mówiąc już o pamięci komputera:

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);
    }
}

wynikowa IL:

.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
}

Niezależnie od tego, czy używasz stałej, literału, czy kilobajta kodu przy użyciu stałej arytmetyki, wartość jest traktowana dosłownie w IL.

Powiązany punkt: stałe składanie dotyczy literałów łańcuchowych. Wielu uważa, że ​​takie wywołanie powoduje zbyt wiele niepotrzebnych, nieefektywnych połączeń łańcuchowych:

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;
    }
}

Ale sprawdź 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

Konkluzja: Operatory na wyrażeniach stałych dają w wyniku wyrażenia stałe, a kompilator wykonuje wszystkie obliczenia; nie wpływa na wydajność w czasie wykonywania.