Lisible ou pas: C# plusieurs opérateurs ternaires + Jeter si inégalée [fermé]
avez-vous trouvé le code C# suivant lisible?
private bool CanExecuteAdd(string parameter) {
return
this.Script == null ? false
: parameter == "Step" ? true
: parameter == "Element" ? this.ElementSelectedInLibrary != null && this.SelectedStep != null
: parameter == "Choice" ? this.SelectedElement != null
: parameter == "Jump" ? this.SelectedStep != null
: parameter == "Conditional jump" ? false
: false.Throw("Unknown Add parameter {0} in XAML.".F(parameter));
}
où le lancer est défini comme:
public static T Throw<T>(this T ignored, string message) {
throw new Exception(message);
}
je sais que ce n'est pas idiomatique C#. Cependant, seriez-vous capable de le comprendre à première ou deuxième vue? Ou ai-je errants trop loin?
15 réponses
j'ai utilisé ce genre de code en Java. Je n'aime pas le bit false.Throw
, mais avoir plusieurs conditionnals comme celui-ci (particulièrement formaté de cette façon) est très bien à mon avis.
c'est un peu étrange la première fois que vous le voyez, mais après c'est juste un modèle pratique à connaître.
une alternative à l'utilisation de false.Throw
ici serait quelque chose comme ceci:
bool? ret = this.Script == null ? false
: parameter == "Step" ? true
: parameter == "Element" ? this.ElementSelectedInLibrary != null && this.SelectedStep != null
: parameter == "Choice" ? this.SelectedElement != null
: parameter == "Jump" ? this.SelectedStep != null
: parameter == "Conditional jump" ? false
: null;
if (ret == null)
{
throw new ArgumentException(
string.Format("Unknown Add parameter {0} in XAML.", parameter);
}
return ret.Value;
EDIT: en fait, dans dans ce cas, je n'utiliserais ni si/sinon ou Ce modèle... J'utiliserais switch / case. Cela peut être très compact si vous voulez:
if (this.Script == null)
{
return false;
}
switch (parameter)
{
case "Step": return true;
case "Element": return this.ElementSelectedInLibrary != null && this.SelectedStep != null;
case "Choice": return this.SelectedElement != null;
case "Jump": return this.SelectedStep != null;
default: throw new ArgumentException(
string.Format("Unknown Add parameter {0} in XAML.", parameter);
}
Pourquoi ne pas utiliser un interrupteur? Je pense que c'est plus lisible.
private bool CanExecuteAdd(string parameter) {
if (Script == null)
return false;
switch (parameter) {
case "Step":
return true;
case "Element":
return ElementSelectedInLibrary != null && SelectedStep != null;
case "Choice":
return SelectedElement != null;
case "Jump":
return SelectedStep != null;
case "Conditional jump":
return false;
default:
throw new Exception(string.Format("Unknown Add parameter {0} in XAML.", parameter));
}
}
ma règle de base: utilisez des expressions pour les choses sans effets secondaires. Utilisez les énoncés pour les choses avec un effet secondaire et pour le flux de contrôle.
lancer est effectivement un effet secondaire; il ne calcule pas une valeur, il modifie le flux de contrôle. Vous calculez une valeur, calcul, calcul, calcul, et puis boom, effet secondaire. Je trouve ce code déroutant et vexant. Je dis que le flux de contrôle devrait être dans les déclarations, pas l'effet secondaire de quelque chose qui semble comme un calcul.
je vote pour non lisible.
bien que la syntaxe soit correcte, elle est quelque peu alambiquée et puisque ce n'est pas, si j'ose dire, "traditionnel", de nombreux développeurs devront perdre du temps à essayer de s'assurer qu'ils comprennent ce qu'ils lisent. Pas une situation idéale.
la lisibilité est certainement un ingrédient clé pour un bon codage, et je dirais que votre échantillon n'est pas immédiatement lisible pour la plupart des devs.
j'aime l'opérateur conditionnel, et de l'utiliser beaucoup.
cet exemple est un peu confus, parce que la nature de l'opérateur n'est pas claire de la disposition et de l'usage.
J'aime à tout le moins clarifier le choix et les alternatives en utilisant ce formatage:
choice
? true-case
: false-case
Mais si nous appliquons cela à votre code, il révèle le manque de clarté dans l'utilisation de la construction de cette façon:
return
this.Script == null
? false
: parameter == "Step"
? true
: parameter == "Element"
? this.ElementSelectedInLibrary != null && this.SelectedStep != null
: parameter == "Choice"
? this.SelectedElement != null
: parameter == "Jump"
? this.SelectedStep != null
: parameter == "Conditional jump"
? false
: false.Throw("Unknown Add parameter {0} in XAML.".F(parameter));
j'ai l'impression que nous essayons d'utiliser l'opérateur conditionnel comme un énoncé d'interrupteur, où un énoncé d'interrupteur, ou mieux encore un modèle de conception comme le modèle de commande serait beaucoup plus clair.
je n'aime vraiment pas ce code. Il m'a fallu plus de 15 secondes à comprendre, donc j'ai renoncé.
un if/then serait préférable.
Convertissez votre ternaire emboîté en interrupteur. Ne forcez jamais une structure de contrôle à faire de façon médiocre ou illisible ce qu'une structure intégrée fera parfaitement, surtout s'il n'y a aucun avantage évident.
être non idiomatique signifie que vous forcez le lecteur à passer du temps à se demander si ce qu'il lit signifie ce qu'il pense que cela signifie.
ainsi être lisible n'achète pas le lecteur sophistiqué (à savoir, suspect) beaucoup. Cela me semble être un cas d'être intelligent pour le bien d'être intelligent.
Est-il une raison de ne pas utiliser un interrupteur ou d'une autre, s'construire ici?
pourquoi ne pas utiliser un outil de type nul?
private bool? CanExecuteAdd(string parameter) {
return
this.Script == null ? false
: parameter == "Step" ? true
: parameter == "Element" ? this.ElementSelectedInLibrary != null && this.SelectedStep != null
: parameter == "Choice" ? this.SelectedElement != null
: parameter == "Jump" ? this.SelectedStep != null
: parameter == "Conditional jump" ? false
: null;
}
au début, j'ai été horrifié, mais en fait je ne peux pas penser à une façon d'écrire aussi plus clair dans C# - j'essayais de penser à quelque chose où vous aviez un éventail de fonctions mappées aux résultats, mais il est devenu encore plus laid.
même si l'analyse des conditionnels réels est rude, il est au moins facile de grok l'intention, bien que je préfère utiliser un bloc d'interrupteur et de gérer tout le reste comme un cas spécial.
trop difficile à lire, attention aux exceptions.
gérer chaque cas dans son propre si, alors vous pouvez avoir des conditions plus complexes.
C'est l'une des rares fois, ce nombre de déclarations distinctes dans une méthode, il serait acceptable de
private bool CanExecuteAdd(string parameter)
{
if (this.Script == null)
return false;
if (parameter.NotIn([] {"Step", "Element", "Choice", "Jump", "Conditional jump"})
throw new Exception("Unknown Add parameter {0} in XAML.".F(parameter));
if (parameter == "Step")
return true;
if (parameter == "Element")
this.ElementSelectedInLibrary != null && this.SelectedStep != null;
// etc, etc
}
Oh, et le .NotIn est une méthode d'extension, le contraire de cela, j'imagine (ne peut pas dire que ce soit tout à fait exact à ce qui est nécessaire)
public static bool In<T>(this T obj, IEnumerable<T> arr)
{
return arr.Contains(obj);
}
ça me semble bien, mais je changerais ta méthode de lancer pour:
static TObject Throw<TObject, TException>(this TObject ignored, TException exception)
{
throw exception;
}
qui vous permet de spécifier le type d'Exception lancée.
Malheureusement, l'opérateur ternaire (?:) n'est pas un idiome commun dans le C langues--j'ai rencontré beaucoup de C, C++ et C# développeurs qui ont de pause pour le lire parce qu'ils ne sont pas familier avec elle ou ne l'utilisez pas. Cela n'en fait pas une mauvaise caractéristique de langage ou illisible, mais ces mêmes développeurs peuvent appeler l'exemple d'OP illisible parce qu'il intègre une caractéristique de langage qui les met mal à l'aise.
Je ne trouve pas l'exemple illisible--j'ai vu des ternaires imbriqués opérateurs plusieurs fois. Cependant, je pense que l'utilisation d'un commutateur serait un choix préférable pour vérifier le "paramètre" par rapport aux chaînes.
pour moi, la méthode de L'extension de jet qui ignore le paramètre 'ceci' est beaucoup plus gênante. Ce qui ferait 42.Jeter.(..)? Si je revoyais le code, je le qualifierais de mauvais design.
dans C & C++, l'utilisation décrite est idiomatique et la raison de l'opérateur. L'avantage du conditionnel ternaire vs. un enchaîné Si-puis-autrement est que c'est une expression avec un type connu. En C++, vous pouvez en fait écrire
foo_t foo = bar_p ? bar
: qux_p ? qux
: woz_p ? woz
: (throw SomeExpression(), foo_t())
;
remarquez l'opérateur virgule, qui renvoie un foo_t qui ne sera jamais construit à cause du lancer.
en fait, je n'ai jamais vu l'opérateur ternaire poussé aussi loin. Cependant, j'ai compris où que vous alliez. Je suis D'accord avec Jon, Je n'aime pas les faux.Jeter la partie.