Verificando o resultado de um construtor em C #

Estou trabalhando em uma base de código com um colega de trabalho que tem o hábito de verificar os resultados de um construtor por um nulo de maneira semelhante a esta

Person p = new Person();
if (p != null)
{
    p.Name = "John Smith";
}

Meu entendimento do cenário do .NET é que um construtor nunca deixará uma tarefa não realizada, a menos que uma exceção seja lançada. Portanto, no caso acima, a verificação nula é inútil. Ou pé alocado, ou uma exceção será lançada causando a propriedade setter para ser ignorada.

Eu perguntei ao meu colega de trabalho sobre isso de passagem e recebo uma resposta passiva na linha de "por precaução". Eu não gosto desse tipo de "programação de parônia". Eu acho que isso prejudica a legibilidade e aumenta desnecessariamente a complexidade ciclomática. Por esse motivo, gostaria de fazer um pedido formal para que essa prática seja interrompida. Este é um pedido razoável? Estou esquecendo de algo?

Eu concordo com @MartinMaat sobre escolher suas batalhas.

Existem muitos casos de "just in case" devido ao fato de não entender o idioma, apesar de o idioma estar fixado em suas regras para muitas dessas coisas - entre parênteses uma expressão que não é necessária por não entender as regras de precedência de o idioma. Mas ainda assim, essa prática é principalmente inofensiva.

Quando eu era jovem, achava que deveríamos aprender os detalhes do idioma e, assim, evitar escrever esse código supérfluo. (Uma de minhas irritações foi return (0);com suas parênteses desnecessárias.) No entanto, agora eu modero essa posição, em particular, porque agora usamos tantos idiomas diferentes, saltando de cliente para servidor, etc. folga para alguns desses problemas.

Seu ponto de vista sobre o início ciclomático é o argumento logicamente fundamentado. Vejamos a Cobertura de código e os níveis de cobertura especialmente mais altos :

2. Cada decisão leva todos os resultados possíveis

Como não podemos forçar a nova operação a retornar NULL, não há como atingir níveis mais altos de cobertura de código para esta operação condicional.   Obviamente, isso pode ou não ser importante para sua organização!

No entanto, devido a esse problema de cobertura do código, eu o priorizaria mais do que entre parênteses.

Por outro lado, o código gerado subjacente provavelmente não sofrerá um pouco com isso, pois as gerações de código, JIT e otimizadores entendem que um newvalor ed nunca será nulo. Portanto, o custo real vem apenas em termos de legibilidade e recursos de cobertura do código fonte.

Gostaria de perguntar como é a "parte mais" de tal declaração if?

Se não houver outra parte, eu argumentaria que simplesmente cair no final da rotina ou passar para outro código (ou seja, não elsepara isso if) é potencialmente perigoso, pois agora esse "apenas no caso" sugere logicamente que os chamadores e / ou outro código abaixo da linha também lida com NULL.

Se estiver escrito:

p = new Object ();
if ( p != null ) {
    p.field = value;
}
else {
    throw new NullReferenceException ();
}

então isso é realmente um exagero, pois a linguagem faz tudo isso por nós.

Eu poderia sugerir reverter o sentido do condicional - talvez seu colega se sinta mais confortável com isso:

p = new Object ();
if ( p == null ) {
    throw new NullReferenceException ();
}
else {
    p.field = value;
}

Agora você pode defender a remoção do wrapper else, pois é claramente desnecessário:

p = new Object ();
if ( p == null ) {
    throw new NullReferenceException ();
}
p.field = value;

Com isso, o "apenas no caso" agora é o que é condicional, enquanto o código seguinte não é. Essa abordagem reforça ainda mais que, quando a alocação falha, a resposta apropriada é lançada, em vez de continuar executando o código nesse método e / ou nessa cadeia de chamadas (sem qualquer outro tratamento adequado da falha na alocação).

Portanto, em resumo, existem dois argumentos logicamente fundamentados a serem apresentados aqui contra essa prática:

1. Não é possível alcançar níveis mais altos de cobertura de código, pois não podemos forçar a falta de memória (ou qualquer falha no construtor) para retornar nulo.
2. O "just in case" (como mostrado acima na pergunta) está incompleto e, como tal, é defeituoso devido à inconsistência nas expectativas de como o nulo deveria ser tratado por outro código além / além do p.field = value;.

Fundamentalmente, parece que talvez seu colega esteja em dúvida sobre o uso de exceções - mesmo que não haja escolha aqui em C # para essas coisas. ( Se queremos código bem testado , não podemos codificar um modelo de exceção para manipular um modelo nulo e um não-exceção usando valores de retorno nulo, lado a lado.) Talvez se você argumentar com seu colega sobre estes tópicos, eles verão alguma luz!

Talvez você possa evitar o problema e simplesmente informar que existe uma sintaxe c # mais nova que é mais fácil (e faz a verificação nula para você!)

Ao invés de

Person p = new Person();
if (p != null)
{
    p.Name = "John Smith";
}

Escreva

var p = new Person
{
    Name = "John Smith"
};

É um pedido razoável, mas parece que isso já se tornou uma luta entre você e ele. Você já deve ter apontado para ele, ele está relutante em mudar isso e agora planeja escalar o assunto. Então você pode "vencer".

Pode ser mais fácil enviar o link que Robert Harvey postou e dizer "Enviei um link sobre a construção de objetos em C # que você pode achar interessante" e deixá-lo lá. Não é exatamente prejudicial o que ele faz (apenas sem sentido) e fácil de limpar depois. Estou dizendo: escolha suas batalhas. Estive nessa situação mais de uma vez e, em retrospecto, muitas vezes simplesmente não valia a pena a luta. Vocês facilmente se odeiam por praticamente nada. Sim, você está certo, mas, embora seja apenas você e ele, convém preservar um terreno comum.

Como sua pergunta técnica já foi respondida (a verificação nula é inútil), gostaria de focar em um aspecto diferente da sua pergunta - como lidar com uma situação desse tipo em geral: um colega de trabalho que não conhece alguns fatos básicos sobre as ferramentas usadas por todos e, portanto, usa as ferramentas de maneira menos que ótima.

Em alguns casos, isso pode não afetá-lo muito. Se eles realizarem o trabalho e não interferirem no seu, você pode simplesmente ignorar o problema.

Em outros casos (por exemplo, o seu), eles não interferir com o seu trabalho, pelo menos até certo ponto. O que fazer sobre isso?

Eu acho que depende de muitos detalhes. Por exemplo, há quanto tempo você e seu colega de trabalho estão com a empresa, como a empresa geralmente lida com essas questões, quanto o problema afeta / incomoda você, como você se dá bem com o colega de trabalho, qual a importância de uma harmonia. relacionamento com seu colega de trabalho, em que medida a abertura ou a escalada do problema pode afetar esse relacionamento etc.

Minha opinião pessoal pode ser a seguinte: acho que os engenheiros devem conhecer suas ferramentas e os engenheiros de software devem conhecer suas linguagens de programação. É claro que ninguém conhece todos os detalhes, mas que os construtores nunca retornam nullé uma ideia muito básica - todos devem saber. Quando preciso trabalhar em algum código que contenha verificações nulas inúteis, simplesmente as remova. Quando um colega me pergunta por que os removi, explico por que eles não fazem sentido. (Se necessário, eu também explicaria por que o código que é totalmente desnecessário deve ser removido.) A longo prazo, isso levaria ao código livre dessas verificações nulas inúteis.

Isso pode parecer um pouco arrogante e pode não ser a abordagem certa para todos. Disseram-me que sou paciente e bom em explicar as coisas. Talvez seja por isso que tendem a me safar dessa abordagem sem parecer um idiota. :-)

Você menciona 'uma solicitação formal para que essa prática seja interrompida'. Eu nunca trabalhei em uma empresa que tivesse regras formais detalhadas, mas aqui estão algumas reflexões. Mais uma vez, acho que seu melhor curso de ação depende dos detalhes. Se o preenchimento de uma solicitação desse tipo deixasse seu colega de trabalho ruim aos olhos de colegas ou gerentes, provavelmente não o faria. Por outro lado, se essas solicitações geralmente são vistas como uma melhoria bem-vinda e outras pessoas não culparão seu colega de trabalho por não seguir essa regra anteriormente, vá em frente. Ninguém será prejudicado, todos estarão em melhor situação.

Mas se eu não conseguir encontrar uma boa maneira de lidar com o problema, posso recorrer ao sarcasmo e adicionar código como este em qualquer lugar:

int c = a + b;
if (c == a + b)
{
    // next steps
}
else 
{
    throw new ArithmeticException();
}

É tão inútil quanto uma verificação nula após uma chamada de construtor, mas ainda mais óbvio. E, claro, eu realmente não faria isso. A menos que eu esteja tão irritado com meus colegas de trabalho que planejo desistir de qualquer maneira. ;-)