¿Es esta una situación correcta para usar una constante?

42

Entonces, mi profesor estaba devolviendo algunos comentarios sobre un proyecto en el que he estado trabajando. Él acopló algunas marcas para este código:

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

Esto está en un controlador de "índice cambiado" de cuadro combinado. Se usa cuando el usuario desea crear un nuevo proveedor, mi opción principal (índice 0, que nunca cambia) abre el cuadro de diálogo "Crear un nuevo proveedor". Entonces, el contenido de mi cuadro combinado se ve así:

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

Su problema es con el código de la primera línea:

if (comboVendor.SelectedIndex == 0)

Afirma que el 0 debería ser una constante, y en realidad me acopló marcas debido a eso. Afirma que no debería usar literales en mi código en absoluto.

El problema es que no entiendo por qué querría hacer que ese código en esa situación sea una constante. Ese índice nunca cambiará, ni es algo que deba modificar. Parece un desperdicio de memoria mantener un solo 0 en la memoria que se usa para una situación muy específica y nunca cambia.

    
pregunta Red 5 01.12.2011 - 21:56

14 respuestas

90

La forma correcta real de hacer esto en C # es no confiar en el orden de los ComboItems en absoluto .

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
        }
    }
}
    
respondido por el BlueRaja - Danny Pflughoeft 01.12.2011 - 23:53
83

El orden en el cuadro combinado podría cambiar. ¿Qué sucede si agrega otra opción como "Crear proveedor especial ..." antes de su "Crear nuevo vendedor ..."

La ventaja de usar una constante es que si hay muchos métodos que dependen del orden del cuadro combinado, solo tienes que cambiar la constante y no todos los métodos si esto cambia.

Usar una constante también es más legible que un literal.

if (comboVendor.SelectedIndex == NewVendorIndex)

La mayoría de los idiomas compilados sustituirán a la constante en el momento de la compilación, por lo que no hay una penalización de rendimiento.

    
respondido por el Michael Krussel 01.12.2011 - 22:09
36

Esta situación que describe es una llamada de juicio, personalmente no usaría una si solo se usa una vez y ya es legible.

Sin embargo, la respuesta real es que lo seleccionó para enseñarte una lección.

No olvide que él es un profesor, su trabajo es enseñarle códigos y mejores prácticas.

Diría que en realidad está haciendo un buen trabajo.

Claro, puede que se salga un poco absoluto, pero estoy seguro de que volverás a pensar antes de usar números mágicos.

También se puso lo suficientemente cerca como para que te unas a una comunidad en línea sobre programadores para descubrir qué se considera una mejor práctica en esta situación.

Felicitaciones a tu profesor.

    
respondido por el Thanos Papathanasiou 01.12.2011 - 22:32
13
  

[...] mi opción superior (índice 0, que nunca cambia) abre el cuadro de diálogo "Crear un nuevo proveedor".

El hecho de que tuvieras que explicar eso prueba por qué debes usar una constante. Si introdujeras una constante como NEW_VENDOR_DIALOG , tu código se explicaría mejor. Además, los compiladores optimizan las constantes, por lo que no habrá un cambio en el rendimiento.

Escribir programas para programadores, no compiladores. A menos que esté tratando específicamente de micro-optimizar, lo que no parece serlo.

    
respondido por el kba 01.12.2011 - 23:15
12
  

Afirma que el 0 debería ser una constante, y en realidad me acopló marcas debido a eso.

Estoy de acuerdo. El uso del cero aquí es "magia". Imagina que estás leyendo este código por primera vez. No sabe por qué el cero es especial, y el literal no le dice nada sobre por qué el cero es especial. Si, por el contrario, dijiste if(comboVendor.SelectedIndex == CreateNewVendorIndex) , entonces es muy claro para el lector por primera vez lo que significa el código.

  

Afirma que no debería usar literales en mi código en absoluto.

Esa es una posición extrema; una posición realista sería decir que el uso de literales es una bandera roja que indica que el código podría no ser tan claro como podría ser. A veces es apropiado.

  

El problema es que no entiendo por qué querría hacer que ese código en esa situación sea una constante. Ese índice nunca cambiará

Que nunca cambiará es una excelente razón por la cual es una constante . Es por eso que las constantes se llaman constantes; porque nunca cambian.

  

ni es algo que necesitarías modificar.

¿De verdad? ¿No puedes ver cualquier situación en la que alguien quiera cambiar el orden de las cosas en un cuadro combinado?

El hecho de que pueda ver una razón por la cual esto podría cambiar en el futuro es una buena razón para no hacer que sea una constante. Más bien debería ser un campo entero estático de solo lectura no constante. Una constante debe ser una cantidad que se garantiza que permanezca igual para siempre . Pi y el número atómico del oro son buenas constantes. Los números de versión no lo son; Cambian todas las versiones. El precio del oro es obviamente una constante terrible; cambia cada segundo Solo haga constantes las cosas que nunca, nunca cambiarán .

  

Parece un desperdicio de memoria mantener un solo 0 en la memoria que se usa para una situación muy específica y nunca cambia.

Ahora llegamos al punto crucial del asunto.

Esta es quizás la línea más importante en su pregunta porque indica que tiene una comprensión muy errónea de (1) la memoria y (2) la optimización. Estás en la escuela para aprender, y ahora sería un buen momento para obtener una comprensión correcta de los fundamentos. ¿Puede explicar en detalle por qué cree que "es un desperdicio de memoria mantener un solo cero en la memoria"? En primer lugar, ¿por qué cree que es relevante optimizar el uso de cuatro bytes de memoria en un proceso con al menos dos mil millones de bytes de almacenamiento direccionable por el usuario? En segundo lugar, ¿qué recurso necesita exactamente? ¿Imagínate se está consumiendo aquí ? ¿Qué quiere decir con "memoria" que se consume?

Primero me interesan las respuestas a estas preguntas porque son una oportunidad para que aprendas cómo tu comprensión de la optimización y el manejo de la memoria es incorrecta, y segundo porque siempre quiero saber por qué los principiantes creen cosas extrañas, para que Puedo diseñar mejores herramientas para que tengan creencias correctas.

    
respondido por el Eric Lippert 02.12.2011 - 18:20
6

Tiene razón. Tienes razón. Te equivocas.

Conceptualmente, tiene razón, que los números mágicos deben evitarse. Las constantes hacen que el código sea más legible al agregar contexto a lo que significa el número. En el futuro, cuando alguien lea su código, sabrán por qué se usó un número en particular. Y si necesita cambiar un valor en algún lugar de la línea, es mucho mejor cambiarlo en un lugar en lugar de intentar buscar en cualquier lugar donde se use un número en particular.

Dicho esto, tienes razón. En este caso específico, realmente no creo que una constante esté justificada. Está buscando el primer elemento de la lista, que siempre es cero. Nunca será 23. O -pi. Estás buscando específicamente el cero. Realmente no creo que necesites desordenar el código haciéndolo una constante.

Sin embargo, se equivoca al suponer que una constante se transporta como una variable, 'usando memoria'. Una constante está ahí para el humano y el compilador. Le dice al compilador que ponga ese valor en ese lugar durante la compilación, donde de otra manera habrías puesto un número literal. E incluso si tuviera la constante en la memoria, para todas las aplicaciones excepto para las más exigentes, la pérdida de eficiencia ni siquiera sería medible. Preocuparse por el uso de la memoria de un solo entero definitivamente cae en una "optimización prematura".

    
respondido por el GrandmasterB 01.12.2011 - 22:52
2

Habría reemplazado el 0 con una constante para aclarar el significado, como NewVendorIndex . Nunca se sabe si su orden cambiará.

    
respondido por el Bernard 01.12.2011 - 22:11
1

Esa es la preferencia total de tu profesor. Por lo general, solo se usa una constante si el literal se usará varias veces, se desea que sea obvio para el lector cuál es el propósito de la línea, o posiblemente se modifique su literal en el futuro, y solo desea cambiar en un solo lugar Sin embargo, para este semestre, el profesor es el jefe, así que lo haría de ahora en adelante en esa clase.

¿Buena formación para el mundo corporativo? Muy Posiblemente.

    
respondido por el Jonathan Henson 01.12.2011 - 22:10
1

Para ser honesto, aunque no creo que su código sea la mejor práctica, su sugerencia es francamente un poco grotesca.

Una práctica más común para un cuadro combinado de .NET es dar un valor vacío al elemento "Seleccionar ...", mientras que los elementos reales tienen valores significativos, y luego hacer:

if (string.IsNullOrEmpty(comboVendor.SelectedValue))

en lugar de

if (comboVendor.SelectedIndex == 0)
    
respondido por el Carson63000 01.12.2011 - 23:01
1

Él no está equivocado al enfatizar el valor de usar constantes y usted no está equivocado al usar literales. A menos que haya enfatizado que este es el estilo de codificación esperado, no debe perder marcas por el uso de literales, ya que no son dañinos. He visto literales utilizados en todo el lugar tantas veces en código comercial.

Su punto es bueno, sin embargo. Esto puede ser a su manera para hacerle conocer los beneficios de las constantes:

1-Protegen su código en cierta medida de manipulaciones accidentales

2-Como @DeadMG dice en su respuesta, si el mismo valor literal se usa en muchos lugares, puede aparecer con un valor diferente por error: las constantes conservan la consistencia.

Las 3 constantes conservan el tipo, por lo que no tienes que usar algo como 0F para significar cero.

4-Para facilitar la lectura, COBOL usa CERO como una palabra reservada para el valor cero (pero también le permite usar el cero literal). Por lo tanto, dar un valor a un nombre a veces es útil, por ejemplo: (Fuente: 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;
}

o como en su caso (como se muestra en la respuesta de @Michael Krussel)

    
respondido por el NoChance 01.12.2011 - 22:23
0

Solo necesitarías ponerlo en una constante si tiene una derivación compleja, o si se repite con frecuencia. Si no, un literal está bien. Poner todo en una constante es un exceso excesivo.

    
respondido por el DeadMG 01.12.2011 - 22:02
0

En realidad, como se mencionó, ¿qué pasa si la posición cambia? Lo que podría / debería hacer es usar un código, en lugar de confiar en el índice.

Entonces, cuando creas tu lista de selección, termina con html como

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

Luego, en lugar de verificar selectedIndex === 0 , verifique que el valor sea CREATECODE , que estaría en una constante y se habría utilizado para esta prueba, y al crear la lista de selección.

    
respondido por el CaffGeek 01.12.2011 - 23:04
0

Me libraría de ello por completo. Simplemente coloque un nuevo botón de crear junto a la lista del cuadro combinado. Haga doble clic en un elemento de la lista para editar o haga clic en el botón. No tienes la nueva funcionalidad enterrada en el combobox. Entonces el número mágico se elimina por completo.

En general, cualquier número literal en el código debe definirse como una constante para poner contexto alrededor del número. ¿Qué significa cero? En este caso 0 = NEW_VENDOR. En otros casos, podría significar algo diferente, por lo que siempre es una buena idea para la legibilidad y la capacidad de mantenimiento poner un poco de contexto a su alrededor.

    
respondido por el Jon Raynor 01.12.2011 - 23:05
0

Como han dicho otros, debe usar algún método distinto del número de índice para identificar el elemento del cuadro combinado que corresponde a una acción determinada; o, puede encontrar el índice con alguna lógica programática y almacenarlo en una variable.

La razón por la que escribo es para abordar su comentario sobre el "uso de la memoria". En C #, como en la mayoría de los idiomas, el compilador "dobla" las constantes. Por ejemplo, compile el siguiente programa y examine el IL. Encontrará que todos esos números ni siquiera llegan a la IL, y mucho menos a la memoria de la computadora:

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

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

Por lo tanto, si usa una constante, un literal o un kilobyte de código usando aritmética constante, el valor se trata literalmente en la IL.

Un punto relacionado: el plegado constante se aplica a los literales de cadena. Muchos creen que una llamada como esta provoca demasiada concatenación de cadenas innecesaria e ineficiente:

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

Pero echa un vistazo a la 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

Línea inferior: los operadores en expresiones constantes dan como resultado expresiones constantes, y el compilador realiza todos los cálculos; no afecta el rendimiento en tiempo de ejecución.

    
respondido por el phoog 23.12.2011 - 20:06

Lea otras preguntas en las etiquetas