Em performance não consigo ver nada que possa ajudar muito, mas este tipo de coisa só dá para ter certeza fazendo testes em condições reais. A não ser que esteja tendo problemas reais, eu não me preocuparia muito com isto. Eu sempre procuro fazer o mais rápido quando isto é bem óbvio, mas não se ficar ilegível, se o ganho for marginal e desnecessário. Certamente tem algumas coisas aí que poderia dar ganhos marginais mas duvido que algo que compense.
Em legibilidade tem várias coisas que poderiam ser melhoradas, mas nada muito importante. Algumas talvez seja mais gosto.
Eu não uso variável quando ela só vai ser usada uma vez a não ser que isto contribua para a legibilidade, neste caso acho que retornar a expressão direto é melhor.
Por outro lado, no primeiro método tem algumas chamadas de métodos mais de uma vez. Provavelmente - mas não certamente - eu chamaria e guardaria em variável e usaria ela. Isto provavelmente melhoraria a performance, marginalmente.
Este método também está um pouco difícil de ler por ter operadores ternários condicionais aninhados. Dá para entender mas poderia pelo menos organizar melhor. Mesmo não precisando tanto, os demais estão mais organizados e cada operando está mais óbvio.
Algumas pessoas recomendariam você usar if
no lugar do operador ternário. Ainda mais que você usa variável. Com ela, até faz algum sentido mesmo, por haver efeito colateral. Olhando o código acho uma boa.
No primeiro método talvez usasse IsEmpty()
já que eu sei que não pode retornar nulo.
Talvez eu repensasse todo este design. Mas não sei se dá por não conhecer todos os detalhes do caso, e não é o foco da pergunta.
Acho uma bobagem usar string.Empty
. Tem quem goste, tem quem ache mais legível, eu não, prefiro ""
. Não tem vantagem alguma usá-lo. Nem desvantagem.
Provavelmente eu usaria o operador +
para concatenar os textos. O compilador irá transformar eles em Concat
e eu acho mais legível com os operadores. Tem quem discorde. Eu acho que esta é a parte mais importante da pergunta para o AP. Mas no fundo, não faz diferença.
Se está pensando que precisa usar StringBuilder
, neste caso não é necessário. Saiba que o Concat
provavelmente fará uso de um StringBuilder
se for necessário. O compilador é inteligente para achar a melhor solução neste caso específico. O assunto já foi tratado em outra pergunta.
Algumas pessoas vão preferir o uso do string.Format()
. De fato aí é caso para pensar nele. Exemplo:
string.Format("{0}.{1}.{2}-{3}", re.CodOrgaoFpe, re.CodUoFpe, re.CodUeFpe, re.NomeUeFpe)
Ou em C# 6:
$"{re.CodOrgaoFpe}.{re.CodUoFpe}.{re.CodUeFpe}-{re.NomeUeFpe}"
Outra coisa que eu provavelmente pensaria em fazer é encapsular estas condições complexas em métodos específicos que retornem um bool
. Assim dá nome para esta condição, não fica parecendo uma coisa mágica, e dica mais DRY, mais canônico, facilitando a manutenção. Em que classe colocar esses métodos é uma arte :)
Tem outras melhorias possíveis se puder usar C# 6. A verificação de null
pode ser dispensada. Pelo menos tem sintaxe melhor.
Um exemplo do que provavelmente eu faria:
private static string MontarDescricaoOrgaoUoUe(RequisicaoFisica requisicao) {
var texto = "";
if (!string.IsNullOrEmpty(MontarNomeOrgao(requisicao)) texto = MontarNomeOrgao(requisicao);
else if (!string.IsNullOrEmpty(MontarNomeUo(requisicao)) texto = MontarNomeUo(requisicao);
else if (!string.IsNullOrEmpty(MontarNomeUe(requisicao)) texto = MontarNomeUe(requisicao);
return texto;
}
private static string MontarNomeOrgao(RequisicaoFisica re) => re.CodOrgaoFpe != null && re.CodUoFpe == null && re.CodUeFpe == null && !string.IsNullOrEmpty(re.NomeOrgaoFpe) ?
re.CodOrgaoFpe + " - " + re.NomeOrgaoFpe :
"";
Coloquei no GitHub para referência futura.
Pela sua pergunta anterior sobre laços me parece que você gosta de fazer tudo no menor número de linhas possível. Não tente fazer isto. Pode piorar a legibilidade, o entendimento de todos os envolvidos e até piorar a performance.