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!
6 Réponses :
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);
}
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);
}
}
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.)
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);
}
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);
}
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);
Pourquoi testez-vous chacun d'eux si vous allez faire la même chose de toute façon