1
votes

Faire la même chose dans diverses instructions «si» peut être amélioré?

J'étais en train de coder quand j'ai rencontré ce problème et j'aimerais savoir si cela est correct ou s'il peut être amélioré pour des raisons de performances:

if (color == 0) {
    if (c.val[0] >= 0 && c.val[0] <= 64) {
        //Black
        Paint(cursor.x + x, cursor.y + y);
    }
}
else if (color == 1) {
    if (c.val[0] >= 64 && c.val[0] <= 128) {
        //Grey
        Paint(cursor.x + x, cursor.y + y);
    }
}
else if (color == 2) {
    if (c.val[0] >= 128 && c.val[0] <= 192) {
        //White Gray
        Paint(cursor.x + x, cursor.y + y);
    }
}

Comme vous pouvez le voir, je fais exactement la même chose chose à l'intérieur de toutes les instructions IF et ça a l'air un peu bizarre, le programme fonctionne comme je veux mais je pense vraiment que je manque quelque chose ..

Merci!


1 commentaires

Pourquoi testez-vous chacun d'eux si vous allez faire la même chose de toute façon


6 Réponses :


2
votes

Quelques opérations logiques le rendent plus compact.

        if ((color == 0 && c.val[0] >= 0 && c.val[0] <= 64) ||
            (color == 1 && c.val[0] >= 64 && c.val[0] <= 128) ||
            (color == 2 && c.val[0] >= 128 && c.val[0] <= 192)) {
                Paint(cursor.x + x, cursor.y + y);
        }


0 commentaires

5
votes

En raison de la structure,

if (color >= 0 && color <= 2) {
    if (c.val[0] >= 64*color && c.val[0] <= 64*(color+1)) {
        Paint(cursor.x + x, cursor.y + y);
    }
}


0 commentaires

0
votes

Ce n'est pas trop terrible avec un morceau de code aussi court répété, mais dans l'intérêt de Ne vous répétez pas, et au cas où plus de code serait nécessaire plus tard, vous pouvez simplement séparer les tests pour savoir si quelque chose sera fait du fait de le faire, en utilisant un indicateur.

if (color >= 0 && color <= 2 && // (if these aren't already guaranteed)
    c.val[0] >= 64*color && c.val[0] <= 64*(color+1)) {
    Paint(cursor.x + x, cursor.y + y);
}

(Notez que les optimisations d'un compilateur peuvent éventuellement gérer ce genre de chose sans réellement mettre un bool en mémoire, juste sauter à l'instruction d'appel de fonction si la condition appropriée est vraie.)

(Cela ressemble également au type de modèle souvent écrit comme un commutateur . Vous pouvez envisager d'utiliser cette syntaxe au lieu de tous les sis.)

Mais en fait, ce code particulier peut devenir encore plus simple, car il existe un modèle mathématique simple pour les trois tests différents que vous effectuez: p >

bool need_Paint = false;
if (color == 0) {
    // Black?
    need_Paint = (c.val[0] >= 0 && c.val[0] <= 64);
}
else if (color == 1) {
    // Grey?
    need_Paint = (c.val[0] >= 64 && c.val[0] <= 128);
}
else if (color == 2) {
    // White Gray?
    need_Paint = (c.val[0] >= 128 && c.val[0] <= 192);
}

if (need_Paint)
    Paint(cursor.x + x, cursor.y + y);

(Une autre chose à considérer pourrait être d'utiliser enum ColorType {BLACK = 0, GREY = 1, WHITE_GRAY = 2}; au lieu d'écrire simplement la "magie chiffres " 0 , 1 et 2 directement. Mais si vous utilisez à la fois un enum et la version mathématique de ce code, je vous recommande de spécifier les valeurs exactes comme indiqué, même si la valeur par défaut pour un enum est toujours comptage consécutif à partir de zéro, pour indiquer clairement que vous comptez sur ces valeurs.)


0 commentaires

0
votes

J'aime votre message original plus que la sélection.

Voici une autre approche avec à peu près le même nombre de lignes ... mais je trouve plus lisible.

void yourSnippet() {
     switch (color) {
     case 0: { PaintIf(  0,  64); } break; // black
     case 1: { PaintIf( 64, 128); } break; // Grey
     case 2: { PaintIf(128, 192); } break; // White Gray
     } // switch
  }

void PaintIf(int min, int max) {
     if ((c.val[0] >= min) && (c.val[0] <= max)) 
        Paint(cursor.x + x, cursor.y + y);
  }


0 commentaires

0
votes

Une autre solution qui suit de près les lignes de 2785528 ​​ 's réponse mais utilise un lambda pour éviter de passer des arguments supplémentaires à PaintIf:

const auto PaintIf = [&](int min, int max)
{
    if (min <= c.val[0] && c.val[0] <= max) 
        Paint(cursor.x + x, cursor.y + y);
};

switch (color)
{
    case 0: PaintIf(  0,  64); break;
    case 1: PaintIf( 64, 128); break;
    case 2: PaintIf(128, 192);
}


0 commentaires

0
votes

Les lambdas sont (à partir de c ++ 17) garantis pour être constexpr (c'est-à-dire des abstractions à coût nul) si possible.

Ils peuvent parfois permettre des expressions logiques plus succinctes et maintenables:

auto range = [](int color)
{
    auto min = color * 64;
    auto max = min + 64;
    return std::make_tuple(min, max);
};

auto in_range = [](auto val, auto tuple)
{
    auto [min, max] = tuple;
    return val >= min && val <= max;
};

if (in_range(c.val[0], range(color)))
    Paint(cursor.x + x, cursor.y + y);


0 commentaires