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?

33
demandé sur Mechanical snail 2010-02-23 20:41:21

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);
}
26
répondu Jon Skeet 2010-02-23 17:43:05

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));
    }
}
29
répondu Fede 2010-02-23 17:57:13

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.

9
répondu Eric Lippert 2010-02-23 21:10:07

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.

8
répondu KP. 2010-02-23 17:47:35

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.

6
répondu John Weldon 2017-05-23 12:02:39

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.

5
répondu David Morton 2010-02-23 17:46:31

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.

4
répondu Alison R. 2010-02-23 17:54:10

ê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?

3
répondu Ori Pessach 2010-02-23 18:11:54

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;

}

2
répondu sylvanaar 2010-02-23 17:48:08

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.

2
répondu Paul Betts 2010-02-23 17:49:24

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);
}
1
répondu CaffGeek 2010-02-23 18:26:33

ç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.

1
répondu Robert Davis 2010-02-23 19:03:37

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.

1
répondu Curt Nichols 2010-02-23 19:26:55

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.

1
répondu Derrick 2011-05-07 16:34:45

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.

0
répondu NotMe 2010-02-23 17:46:50