Lisible ou non: plusieurs opérateurs ternaires C # + Lancer si inégalé

Question

Trouvez-vous 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ù Throw 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 au premier ou au deuxième coup d'œil? Ou suis-je allé trop loin?

Answer 1

Pourquoi ne pas utiliser un interrupteur? Je pense que c'est beaucoup 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));
    }
}
 

Answer 2

J'ai utilisé ce genre de code en Java un montant équitable. Je n'aime pas l' false.Throw peu, mais le fait d'avoir plusieurs conditions comme ceci (en particulier sous la forme) est très bien de mon point de vue.

C'est un peu étrange de la très première fois, vous voyez, mais après c'est juste une pratique modèle à connaître.

Une alternative à l'utilisation d' 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 ce cas, je ne voudrais pas l'utiliser si/d'autre ou ce modèle... je l'avais utiliser le commutateur/cas. 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);
}

Answer 3

Je vote pour non lisible.

Bien que la syntaxe soit correcte, elle est quelque peu compliquée et comme ce n'est pas, oserais-je 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é d'un bon codage, et je dirais que votre échantillon n'est pas immédiatement lisible pour la plupart des développeurs.

Answer 4

Ma règle d'or: utilisez des expressions pour des choses sans effets secondaires. Utilisez des instructions pour des choses avec un effet secondaire et pour contrôler le flux.

Le lancer est effectivement un effet secondaire; il ne calcule pas de valeur, il modifie le flux de contrôle. Vous calculez une valeur, calcul, calcul, calcul, puis boom, effet secondaire. Je trouve le code comme ça déroutant et vexant. Je dis que le flux de contrôle doit être dans les instructions, pas l'effet secondaire de quelque chose qui ressemble à un calcul.

Answer 5

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 clair à partir de la mise en page et l'utilisation.

À tout le moins, je tiens à faire les choix et les alternatives clairement par l'utilisation de cette mise en forme:

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

Cela se sent à moi comme nous essayons d'utiliser l'opérateur conditionnel comme une instruction switch, où une instruction switch, ou mieux encore un modèle de conception comme le Modèle de Commande seront beaucoup plus claires.