Esta é uma situação correta para usar uma constante?

42

Então, meu professor estava dando um feedback sobre um projeto em que estou trabalhando. Ele encaixou algumas marcas para este código:

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

Isso está em um manipulador de "índice alterado" da caixa de combinação. É usado quando o usuário deseja criar um novo fornecedor, minha opção principal (índice 0, que nunca muda) abre a caixa de diálogo "Criar um novo fornecedor". Então o conteúdo da minha caixa de combinação acaba ficando assim:

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

O problema dele é com o código da primeira linha:

if (comboVendor.SelectedIndex == 0)

Ele alega que o 0 deve ser uma constante e, na verdade, atracai minhas marcas por causa disso. Ele afirma que eu não deveria usar literais no meu código.

O problema é que não entendo por que gostaria de tornar esse código nessa situação uma constante. Esse índice nunca vai mudar, nem é algo que você precisa ajustar. Parece um desperdício de memória manter um único 0 na memória que é usado para uma situação muito específica e nunca muda.

    
por Red 5 01.12.2011 / 21:56
fonte

14 respostas

90

A maneira correta de fazer isso em C # é não depender da ordenação dos ComboItems de todo .

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
        }
    }
}
    
por 01.12.2011 / 23:53
fonte
83

A ordem na caixa de combinação pode mudar. E se você adicionar outra opção como "Criar fornecedor especial ..." antes do "Criar novo revendedor ..."

A vantagem de usar uma constante é se houver muitos métodos que dependem da ordem da caixa de combinação, você só precisa alterar a constante e não todos os métodos se isso mudar.

O uso de uma constante também é mais legível que um literal.

if (comboVendor.SelectedIndex == NewVendorIndex)

A maioria das linguagens compiladas substituirá a constante em tempo de compilação, portanto não há penalidade de desempenho.

    
por 01.12.2011 / 22:09
fonte
36

Esta situação que você descreve é uma chamada de julgamento, pessoalmente eu não usaria uma se ela fosse usada apenas uma vez e já estivesse legível.

A verdadeira resposta é que ele escolheu isso para ensinar uma lição.

Não se esqueça de que ele é professor, seu trabalho é ensinar codificação e práticas recomendadas.

Eu diria que ele está fazendo um bom trabalho, na verdade.

Claro que ele pode sair um pouco absoluto, mas tenho certeza que você vai pensar novamente antes de usar números mágicos.

Além disso, ele ficou sob a sua pele o suficiente para você participar de uma comunidade on-line sobre programadores apenas para descobrir o que é considerado uma prática recomendada nessa situação.

Tiremos o chapéu para o seu professor.

    
por 01.12.2011 / 22:32
fonte
13

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

Esse fato que você teve que explicar provou porque você deveria usar uma constante. Se você introduziu uma constante como NEW_VENDOR_DIALOG , seu código seria mais auto-explicativo. Além disso, os compiladores otimizam as constantes, portanto, não haverá uma mudança no desempenho.

Escreva programas para programadores, não para compiladores. A menos que você esteja especificamente tentando otimizar o micro, o que não parece que você é.

    
por 01.12.2011 / 23:15
fonte
12

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

Eu concordaria. O uso de zero aqui é "mágico". Imagine que você está lendo este código pela primeira vez. Você não sabe porque o zero é especial, e o literal não informa nada sobre o porquê de zero ser especial. Se em vez disso você disse if(comboVendor.SelectedIndex == CreateNewVendorIndex) , então fica extremamente claro para o leitor de primeira viagem o que o código significa.

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

Essa é uma posição extrema; uma posição realista seria dizer que o uso de literais é uma bandeira vermelha que indica que o código pode não ser tão claro quanto poderia ser. Às vezes é apropriado.

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

O fato de que isso nunca mudará é uma razão excelente para torná-lo constante . É por isso que constantes são chamadas constantes; porque eles nunca mudam.

nor is it something that you would need to tweak.

Realmente? Você não pode ver qualquer situação em que alguém pode querer mudar a ordem das coisas em uma caixa de combinação?

O fato de você poder ver uma razão pela qual isso pode mudar no futuro é uma boa razão para não torná-lo uma constante. Em vez disso, ele deve ser um campo inteiro estático somente leitura e não constante. Uma constante deve ser uma quantidade garantida para permanecer a mesma para todo o tempo . Pi e o número atômico de ouro são boas constantes. Números de versão não são; eles mudam todas as versões. O preço do ouro é obviamente uma constante terrível; muda a cada segundo. Apenas faça constante coisas que nunca, nunca mudam .

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.

Agora chegamos ao cerne da questão.

Esta é talvez a linha mais importante em sua pergunta, porque indica que você tem uma compreensão profundamente falha da (1) memória e (2) otimização. Você está na escola para aprender e agora seria um ótimo momento para entender corretamente os fundamentos. Você pode explicar em detalhes por que você acredita que "é um desperdício de memória manter um único zero na memória"? Primeiramente, por que você acredita que otimizar o uso de quatro bytes de memória em um processo com pelo menos dois bilhões de bytes de armazenamento endereçável pelo usuário é relevante? Segundo, precisamente qual recurso você usa? imagine está sendo consumido aqui ? O que você quer dizer com "memória" sendo consumida?

Estou interessado nas respostas a estas perguntas primeiro porque elas são uma oportunidade para você aprender como seu entendimento de otimização e gerenciamento de memória está incorreto, e segundo porque eu sempre quero saber porque iniciantes acreditam em coisas estranhas, então Eu posso projetar ferramentas melhores para levá-las a ter crenças corretas.

    
por 02.12.2011 / 18:20
fonte
6

Ele está certo. Você está certo. Você está errado.

Ele está certo, conceitualmente, que números mágicos devem ser evitados. As constantes tornam o código mais legível adicionando contexto ao que o número significa. No futuro, quando alguém lê seu código, eles sabem por que um determinado número foi usado. E se você precisar alterar um valor em algum ponto da linha, é muito melhor alterá-lo em um lugar, em vez de tentar procurá-lo em qualquer lugar em que um determinado número seja usado.

Dito isto, você está certo. Neste caso específico, eu realmente não acho que uma constante é justificada. Você está procurando o primeiro item na lista, que é sempre zero. Nunca será 23. Ou -pi. Você está procurando especificamente por zero. Eu realmente não acho que você precisa desordenar o código, tornando-o uma constante.

Você está errado, no entanto, ao assumir que uma constante é carregada como uma variável, "usando memória". Uma constante está lá para o humano e o compilador. Ele diz ao compilador para colocar esse valor naquele ponto durante a compilação, onde você teria colocado um número literal. E mesmo que carregasse a constante na memória, para todas as aplicações menos exigentes, a perda de eficiência nem seria mensurável. Preocupar-se com o uso de memória de um único inteiro definitivamente cai em 'otimização prematura'.

    
por 01.12.2011 / 22:52
fonte
2

Eu teria substituído o 0 por uma constante para deixar claro o significado, como NewVendorIndex . Você nunca sabe se seu pedido será alterado.

    
por 01.12.2011 / 22:11
fonte
1

Essa é a preferência total do seu professor. Normalmente, você só usa uma constante se o literal for usado várias vezes, você quer tornar óbvio para o leitor qual é o propósito da linha, ou seu literal será possivelmente alterado no futuro, e você só quer mudar em um só lugar. No entanto, para este semestre, o professor é o chefe, então eu faria isso a partir de agora nessa classe.

Boa formação para o mundo corporativo? Muito Possivelmente.

    
por 01.12.2011 / 22:10
fonte
1

Para ser honesto, embora eu não ache que seu código seja a melhor prática, a sugestão dele é francamente um pouco grotesca.

Uma prática mais comum para uma caixa de combinação do .NET é dar ao item "Selecionar ..." um valor vazio, enquanto os itens reais têm valores significativos, e então:

if (string.IsNullOrEmpty(comboVendor.SelectedValue))

em vez de

if (comboVendor.SelectedIndex == 0)
    
por 01.12.2011 / 23:01
fonte
1

Ele não está errado por enfatizar o valor de usar constantes e você não está errado em usar literais. A menos que ele tenha enfatizado que esse é o estilo de codificação esperado, você não deve perder pontos por usar literais, já que eles não são prejudiciais. Eu vi literais usados em todo o lugar tantas vezes no código comercial.

Seu ponto é bom embora. Isso pode ser seu caminho para torná-lo ciente dos benefícios das constantes:

1-Eles protegem seu código até certo ponto de adulteração acidental

2-Como o @DeadMG diz em sua resposta, se o mesmo valor literal é usado em muitos lugares, pode aparecer com um valor diferente por engano - Então, as constantes preservam a consistência.

3-Constantes preservam o tipo, então você não precisa usar algo como 0F para dizer zero.

4-Para facilitar a leitura, o COBOL usa ZERO como uma palavra reservada para o valor zero (mas também permite que você use o zero literal) - Então, dar um valor a um nome às vezes é útil, por exemplo: 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;
}

ou como no seu caso (como mostrado na resposta @Michael Krussel)

    
por 01.12.2011 / 22:23
fonte
0

Você só precisa colocá-lo em uma constante se tiver uma derivação complexa ou se for repetida com frequência. Mais, um literal está bem. Colocar tudo em uma constante é um total exagero.

    
por 01.12.2011 / 22:02
fonte
0

Na verdade, como mencionado, e se a posição mudar? O que você poderia / deveria fazer é usar um código, em vez de confiar no índice.

Então, quando você cria sua lista de seleção, ela acaba com 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>

Então, em vez de verificar selectedIndex === 0 , verifique se o valor é CREATECODE , o que estaria em uma constante e teria sido usado para este teste e ao criar a lista de seleção.

    
por 01.12.2011 / 23:04
fonte
0

Eu me livraria disso completamente. Basta colocar um botão criar novo ao lado da lista da caixa de combinação. Clique duas vezes em um item na lista para editá-lo ou clique no botão. Não tenha a nova funcionalidade enterrada na caixa de combinação. Então o número mágico é removido completamente.

Em geral, qualquer número literal no código deve ser definido como uma constante, de modo a contextualizar o número. O que é zero significa? Nesse caso, 0 = NEW_VENDOR. Em outros casos, isso pode significar algo diferente, por isso é sempre uma boa idéia de legibilidade e manutenção colocar um contexto em torno disso.

    
por 01.12.2011 / 23:05
fonte
0

Como outros já disseram, você deve usar algum método diferente do número do índice para identificar o item da caixa de combinação que corresponde a uma determinada ação; ou, você pode encontrar o índice com alguma lógica programática e armazenar isso em uma variável.

A razão pela qual estou escrevendo é endereçar seu comentário sobre o "uso da memória". Em C #, como na maioria das linguagens, as constantes são "dobradas" pelo compilador. Por exemplo, compile o programa a seguir e examine o IL. Você verá que todos esses números nem chegam ao IL, muito menos a memória do computador:

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

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

Então, se você usa uma constante, um literal ou um kilobyte de código usando aritmética constante, o valor é tratado literalmente no IL.

Um ponto relacionado: O dobramento constante se aplica aos literais de string. Muitos acreditam que uma chamada como essa causa muita concatenação desnecessária e ineficiente de strings:

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

Mas confira o 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

Linha inferior: Operadores em expressões constantes resultam em expressões constantes, e o compilador faz toda a computação; isso não afeta o desempenho em tempo de execução.

    
por 23.12.2011 / 20:06
fonte

Tags