In performance I can not see anything that can help a lot, but this kind of thing can only be sure to do tests under real conditions. Unless you're having real problems, I would not worry about it very much. I always try to do it fast when this is quite obvious, but do not get illegible if the gain is marginal and unnecessary. Certainly there are some things there that could give marginal gains but I doubt something that compensates.
In readability there are several things that could be improved, but nothing very important. Some may taste better.
I do not use a variable when it's only going to be used once unless this adds to the readability, in this case I think returning the direct expression is better.
On the other hand, in the first method you have some method calls more than once. Probably - but certainly not - I would call and keep variable and use it. This would probably improve performance marginally.
This method is also somewhat difficult to read by having nested conditional ternary operators. You can understand but could at least organize better. While not needing it so much, the others are more organized and each operand is more obvious.
Some people would recommend you use if instead of the ternary operator. Even more that you use variable. With it, even makes some sense even because there is side effect. Looking at the code I think a good one.
In the first method I might use IsEmpty() since I know it can not return null.
Maybe I'll rethink all this design . But I do not know if it's because I do not know all the details of the case, and it's not the focus of the question.
I think it's silly to use string.Empty . There are those who like it, there are those who find it more readable, I do not, I prefer "" . There's no use in using it. Neither disadvantage.
I would probably use the + operator to concatenate the texts. The compiler will turn them into Concat and I find it more readable with operators. Some people disagree. I think this is the most important part of the question for the AP. But deep down, it makes no difference.
If you think you need to use StringBuilder , this is not necessary. Note that Concat will probably make use of StringBuilder if needed. The compiler is smart to find the best solution in this specific case. The subject has been addressed in another question .
Some people will prefer using string.Format() . In fact there is case to think about it. Example:
string.Format("{0}.{1}.{2}-{3}", re.CodOrgaoFpe, re.CodUoFpe, re.CodUeFpe, re.NomeUeFpe)
Or in C # 6:
$"{re.CodOrgaoFpe}.{re.CodUoFpe}.{re.CodUeFpe}-{re.NomeUeFpe}"
Another thing I would probably think of doing is to encapsulate these complex conditions into specific methods that return a bool . So it gives name to this condition, it does not look like a magic thing, and it hints at more DRY , more canonical, facilitating maintenance. In which class to place these methods is an art:)
It has other possible improvements if you can use C # 6. The check of null can be dispensed with. At least it has better syntax.
An example of what I would probably do:
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) {
return re.CodOrgaoFpe != null && re.CodUoFpe == null && re.CodUeFpe == null && !string.IsNullOrEmpty(re.NomeOrgaoFpe) ?
re.CodOrgaoFpe + " - " + re.NomeOrgaoFpe :
"";
}
From your previous link question I think you like to do everything in as few rows as possible. Do not try to do this. It can worsen readability, understanding everyone involved and even worsen performance.
