Trop de déclarations «si»?

263

Le code suivant fonctionne comme j'en ai besoin, mais c'est moche, excessif ou plusieurs autres choses. J'ai regardé des formules et tenté d'écrire quelques solutions, mais je me retrouve avec une quantité similaire de déclarations.

Existe-t-il un type de formule mathématique qui me serait utile dans ce cas ou est-ce que 16 si les énoncés sont acceptables?

Pour expliquer le code, c'est pour une sorte de jeu au tour par tour simultané. Deux joueurs ont chacun quatre boutons d'action et les résultats proviennent d'un tableau (0-3), mais les variables «un» et «deux» peuvent être rien attribué si cela aide. Le résultat est, 0 = aucune victoire, 1 = p1 gagne, 2 = p2 gagne, 3 = les deux gagnent.

public int fightMath(int one, int two) {

    if(one == 0 && two == 0) { result = 0; }
    else if(one == 0 && two == 1) { result = 0; }
    else if(one == 0 && two == 2) { result = 1; }
    else if(one == 0 && two == 3) { result = 2; }
    else if(one == 1 && two == 0) { result = 0; }
    else if(one == 1 && two == 1) { result = 0; }
    else if(one == 1 && two == 2) { result = 2; }
    else if(one == 1 && two == 3) { result = 1; }
    else if(one == 2 && two == 0) { result = 2; }
    else if(one == 2 && two == 1) { result = 1; }
    else if(one == 2 && two == 2) { result = 3; }
    else if(one == 2 && two == 3) { result = 3; }
    else if(one == 3 && two == 0) { result = 1; }
    else if(one == 3 && two == 1) { result = 2; }
    else if(one == 3 && two == 2) { result = 3; }
    else if(one == 3 && two == 3) { result = 3; }

    return result;
}
TomFirth
la source
1
@waqaslam: - Cela peut aider l'instruction switch Java à gérer deux variables?
Rahul Tripathi
9
Il y a sûrement une logique ici qui peut être généralisée plutôt que forcée brutalement? Il y a sûrement une fonction f(a, b)qui donne la réponse dans le cas général? Vous n'avez pas expliqué la logique du calcul, donc toutes les réponses ne sont que du rouge à lèvres sur un cochon. Je commencerais par repenser sérieusement la logique de votre programme, l'utilisation d' intindicateurs pour les actions est très dépassée. enums peuvent contenir de la logique et sont descriptifs, cela vous permettrait d'écrire votre code de manière plus moderne.
Boris the Spider
Après avoir lu les réponses @Steve Benett fournies dans sa question alternative liée ci-dessus, je peux supposer qu'il n'y a pas de réponse simple à cette formule car elle est essentiellement la même qu'une base de données. J'ai tenté d'expliquer dans la question d'origine que je faisais un jeu simple (combattant) et les utilisateurs ont une sélection de 4 boutons: blockHigh (0), blockLow (1), attackHigh (2) et attackLow (3). Ces nombres sont conservés dans un tableau jusqu'à ce qu'ils soient nécessaires. Plus tard, ils sont utilisés par la fonction 'fightMath ()' qui appelle les sélections de playerOne contre playerTwos pour donner le résultat. Aucune détection de collision réelle.
TomFirth
9
Si vous avez une réponse, veuillez la publier comme telle. La discussion approfondie dans les commentaires est difficile à suivre, en particulier lorsque le code est impliqué. Si vous souhaitez savoir si cette question aurait dû être migrée vers Code Review, il y a une discussion Meta à ce sujet.
George Stocker
1
Qu'entendez-vous par «la même chose qu'une base de données»? Si ces valeurs se trouvent dans la base de données, extrayez-les de là. Sinon, si c'est vraiment aussi complexe, je le laisserais comme vous l'avez et ajouterais des commentaires sur la logique métier après chaque ligne afin que les gens comprennent ce qui se passe. Il vaut mieux (pour moi) long et explicite - quelqu'un à l'avenir peut comprendre ce qui se passe. Si vous le mettez dans une carte ou essayez d'enregistrer 8 lignes de code, le côté positif est vraiment petit et la réduction est plus grande: vous le rendez de plus en plus déroutant pour quelqu'un qui a besoin de lire votre code un jour.
skaz

Réponses:

600

Si vous ne pouvez pas trouver une formule, vous pouvez utiliser un tableau pour un nombre aussi limité de résultats:

final int[][] result = new int[][] {
  { 0, 0, 1, 2 },
  { 0, 0, 2, 1 },
  { 2, 1, 3, 3 },
  { 1, 2, 3, 3 }
};
return result[one][two];
laalto
la source
7
C'est intéressant car je n'ai jamais vu cette solution auparavant. Je ne suis pas sûr de bien comprendre le résultat du retour, mais j'apprécierai cela.
TomFirth
4
Vous n'avez pas besoin de l'assertion, Java lancera IndexOutOfBoundsExceptionquand même un ou plusieurs index hors limites.
JAB
43
@JoeHarper Si vous voulez quelque chose de facile à lire, vous n'utiliserez pas de nombres magiques en premier lieu et vous aurez un commentaire expliquant le mappage. En l'état, je préfère cette version à l'original, mais pour quelque chose de maintenable à long terme, j'utiliserais une approche impliquant des types énumérés ou au moins des constantes nommées.
JAB
13
@JoeHarper "Théoriquement" est une chose, "pratiquement" en est une autre. Bien sûr, j'essaie d'utiliser des noms descriptifs (à l'exception de la convention i/ j/ kpour les variables de boucle), des constantes nommées, en organisant mon code de manière lisible, etc., mais lorsque les noms de variables et de fonctions commencent à prendre plus de 20 caractères chacun je trouve que cela conduit en fait à du code moins lisible. Mon approche habituelle est d'essayer un code compréhensible mais succinct avec des commentaires ici et là pour expliquer pourquoi le code est structuré tel qu'il est (vs comment). Mettre le pourquoi dans les noms encombre tout.
JAB
13
J'aime cette solution pour ce problème spécifique parce que les résultats SONT réellement dictés par une matrice de résultats.
Essayer
201

Étant donné que votre ensemble de données est si petit, vous pouvez tout compresser en 1 entier long et le transformer en formule

public int fightMath(int one,int two)
{
   return (int)(0xF9F66090L >> (2*(one*4 + two)))%4;
}

Variante plus au niveau du bit:

Cela utilise le fait que tout est un multiple de 2

public int fightMath(int one,int two)
{
   return (0xF9F66090 >> ((one << 3) | (two << 1))) & 0x3;
}

L'origine de la constante magique

Que puis-je dire? Le monde a besoin de magie, parfois la possibilité de quelque chose appelle sa création.

L'essence de la fonction qui résout le problème de OP est une carte de 2 nombres (un, deux), domaine {0,1,2,3} à la plage {0,1,2,3}. Chacune des réponses a abordé la façon de mettre en œuvre cette carte.

En outre, vous pouvez voir dans un certain nombre de réponses une reformulation du problème sous la forme d'une carte de 1 base à 2 chiffres numéro 4 N (un, deux) où un est le chiffre 1, deux est le chiffre 2 et N = 4 * un + deux; N = {0,1,2, ..., 15} - seize valeurs différentes, c'est important. La sortie de la fonction est un nombre de base 4 à 1 chiffre {0,1,2,3} - 4 valeurs différentes, également importantes.

Maintenant, un nombre de base 4 à 1 chiffre peut être exprimé comme un nombre de base 2 à 2 chiffres; {0,1,2,3} = {00,01,10,11}, et ainsi chaque sortie peut être codée avec seulement 2 bits. Par dessus, il n'y a que 16 sorties différentes possibles, donc 16 * 2 = 32 bits est tout ce qui est nécessaire pour coder la carte entière; tout cela peut tenir dans 1 entier.

La constante M est un codage de la carte m où m (0) est codé en bits M [0: 1], m (1) est codé en bits M [2: 3] et m (n) est codé en bits M [n * 2: n * 2 + 1].

Il ne reste plus qu'à indexer et renvoyer la partie droite de la constante, dans ce cas, vous pouvez décaler M vers la droite 2 * N fois et prendre les 2 bits les moins significatifs, c'est-à-dire (M >> 2 * N) & 0x3. Les expressions (un << 3) et (deux << 1) multiplient simplement les choses tout en notant que 2 * x = x << 1 et 8 * x = x << 3.

waTeim
la source
79
intelligent, mais personne d'autre qui lit le code n'aura une chance de le comprendre.
Aran Mulholland
106
Je pense que c'est une très mauvaise pratique. Personne d'autre que l'auteur ne le comprendra. Vous voulez regarder un morceau de code et le comprendre rapidement. Mais ce n'est qu'une perte de temps.
Balázs Németh
14
Je suis avec @ BalázsMáriaNémeth à ce sujet. Bien que très impressionnant, vous devriez coder pour des psychopathes violents!
OrhanC1
90
Tous les downvoters pensent que c'est une odeur hideuse de code. Tous les électeurs pensent la même chose, mais admirent l'intelligence derrière elle. +1 (N'utilisez jamais ce code.)
usr
4
Quel bel exemple d' écriture de code uniquement !
lealand
98

Je n'aime aucune des solutions présentées à l'exception des JAB. Aucun des autres ne facilite la lecture du code et la compréhension de ce qui est calculé .

Voici comment j'écrirais ce code - je ne connais que C #, pas Java, mais vous obtenez l'image:

const bool t = true;
const bool f = false;
static readonly bool[,] attackResult = {
    { f, f, t, f }, 
    { f, f, f, t },
    { f, t, t, t },
    { t, f, t, t }
};
[Flags] enum HitResult 
{ 
    Neither = 0,
    PlayerOne = 1,
    PlayerTwo = 2,
    Both = PlayerOne | PlayerTwo
}
static HitResult ResolveAttack(int one, int two)
{
    return 
        (attackResult[one, two] ? HitResult.PlayerOne : HitResult.Neither) | 
        (attackResult[two, one] ? HitResult.PlayerTwo : HitResult.Neither);
}    

Maintenant, il est beaucoup plus clair ce qui est calculé ici: cela souligne que nous calculons qui est touché par quelle attaque et retournons les deux résultats.

Cependant, cela pourrait être encore mieux; ce tableau booléen est quelque peu opaque. J'aime l'approche de recherche de table, mais je serais enclin à l'écrire de manière à ce que ce soit la sémantique du jeu prévue. C'est-à-dire, plutôt que "une attaque de zéro et une défense de un n'entraîne aucun coup", cherchez plutôt un moyen de faire en sorte que le code implique plus clairement "une attaque à faible coup de pied et une défense à bloc faible n'entraîne aucun coup". Faites en sorte que le code reflète la logique métier du jeu.

Eric Lippert
la source
66
Absurdité. La plupart des programmes moyennement expérimentés pourront apprécier les conseils donnés ici et appliquer le style de codage à leur propre langue. La question était de savoir comment éviter une chaîne de if. Cela montre comment.
GreenAsJade
6
@ user3414693: Je sais bien que c'est une question Java. Si vous lisez attentivement la réponse, cela devient clair. Si vous croyez que ma réponse est imprudente, je vous encourage à écrire votre propre réponse que vous préférez.
Eric Lippert
1
@EricLippert J'aime aussi la solution de JAB. À mon humble avis, le type d'énumération en C # laisse beaucoup à désirer. Il ne suit pas la philosophie du succès que font les autres fonctionnalités. Par exemple stackoverflow.com/a/847353/92414 L'équipe c # a-t-elle un plan pour créer un nouveau type d'énumération (afin d'éviter de casser le code existant) qui est mieux conçu?
SolutionYogi
@SolutionYogi: Je n'aime pas beaucoup non plus les énumérations en C #, bien qu'elles soient telles qu'elles sont pour de bonnes raisons historiques. (Surtout pour la compatibilité avec les énumérations COM existantes.) Je ne suis pas au courant de l'intention d'ajouter de nouveaux équipements pour les énumérations en C # 6.
Eric Lippert
3
@SList non, les commentaires ne s'exécutent pas. OP a fait exactement ce qui devait être fait; convertir les commentaires en code clair. Voir par exemple Steve McConnell Code Complete stevemcconnell.com/cccntnt.htm
djechlin
87

Vous pouvez créer une matrice contenant des résultats

int[][] results = {{0, 0, 1, 2}, {0, 0, 2, 1},{2, 1, 3, 3},{2, 1, 3, 3}};

Lorsque vous voulez obtenir de la valeur, vous utiliserez

public int fightMath(int one, int two) {
  return this.results[one][two]; 
}
djm.im
la source
69

D'autres personnes ont déjà suggéré mon idée initiale, la méthode matricielle, mais en plus de consolider les instructions if, vous pouvez éviter une partie de ce que vous avez en vous assurant que les arguments fournis sont dans la plage attendue et en utilisant des retours sur place (certains codages les normes que j'ai vues appliquent un point de sortie unique pour les fonctions, mais j'ai trouvé que les retours multiples sont très utiles pour éviter le codage des flèches et avec la prévalence d'exceptions en Java, il n'y a pas grand chose à faire appliquer strictement une telle règle de toute façon car toute exception non interceptée levée à l'intérieur de la méthode est un point de sortie possible de toute façon). L'imbrication des instructions de commutateur est une possibilité, mais pour la petite plage de valeurs que vous vérifiez ici, je trouve que les instructions doivent être plus compactes et ne devraient pas entraîner une grande différence de performances,

public int fightMath(int one, int two) {
    if (one > 3 || one < 0 || two > 3 || two < 0) {
        throw new IllegalArgumentException("Result is undefined for arguments outside the range [0, 3]");
    }

    if (one <= 1) {
        if (two <= 1) return 0;
        if (two - one == 2) return 1;
        return 2; // two can only be 3 here, no need for an explicit conditional
    }

    // one >= 2
    if (two >= 2) return 3;
    if (two == 1) return 1;
    return 2; // two can only be 0 here
}

Cela finit par être moins lisible qu'il ne le serait autrement en raison de l'irrégularité de certaines parties du mappage d'entrée-> résultat. Je préfère le style de matrice en raison de sa simplicité et de la façon dont vous pouvez configurer la matrice pour qu'elle ait un sens visuel (bien que cela soit en partie influencé par mes souvenirs des cartes de Karnaugh):

int[][] results = {{0, 0, 1, 2},
                   {0, 0, 2, 1},
                   {2, 1, 3, 3},
                   {2, 1, 3, 3}};

Mise à jour: compte tenu de votre mention de blocage / frappe, voici un changement plus radical de la fonction qui utilise les types énumérés propriété / propriété d'attribut pour les entrées et le résultat et modifie également un peu le résultat pour tenir compte du blocage, ce qui devrait entraîner une plus fonction lisible.

enum MoveType {
    ATTACK,
    BLOCK;
}

enum MoveHeight {
    HIGH,
    LOW;
}

enum Move {
    // Enum members can have properties/attributes/data members of their own
    ATTACK_HIGH(MoveType.ATTACK, MoveHeight.HIGH),
    ATTACK_LOW(MoveType.ATTACK, MoveHeight.LOW),
    BLOCK_HIGH(MoveType.BLOCK, MoveHeight.HIGH),
    BLOCK_LOW(MoveType.BLOCK, MoveHeight.LOW);

    public final MoveType type;
    public final MoveHeight height;

    private Move(MoveType type, MoveHeight height) {
        this.type = type;
        this.height = height;
    }

    /** Makes the attack checks later on simpler. */
    public boolean isAttack() {
        return this.type == MoveType.ATTACK;
    }
}

enum LandedHit {
    NEITHER,
    PLAYER_ONE,
    PLAYER_TWO,
    BOTH;
}

LandedHit fightMath(Move one, Move two) {
    // One is an attack, the other is a block
    if (one.type != two.type) {
        // attack at some height gets blocked by block at same height
        if (one.height == two.height) return LandedHit.NEITHER;

        // Either player 1 attacked or player 2 attacked; whoever did
        // lands a hit
        if (one.isAttack()) return LandedHit.PLAYER_ONE;
        return LandedHit.PLAYER_TWO;
    }

    // both attack
    if (one.isAttack()) return LandedHit.BOTH;

    // both block
    return LandedHit.NEITHER;
}

Vous n'avez même pas besoin de changer la fonction elle-même si vous voulez ajouter des blocs / attaques de plus hauteurs, juste les énumérations; cependant, l'ajout de types de mouvements supplémentaires nécessitera probablement une modification de la fonction. De plus, EnumSets peut être plus extensible que d'utiliser des énumérations supplémentaires en tant que propriétés de l'énumération principale, par exemple EnumSet<Move> attacks = EnumSet.of(Move.ATTACK_HIGH, Move.ATTACK_LOW, ...);et puis attacks.contains(move)plutôt que move.type == MoveType.ATTACK, bien que l'utilisation de EnumSets soit probablement un peu plus lente que les vérifications égales directes.


Dans le cas où un blocage réussi aboutit à un compteur, vous pouvez remplacer if (one.height == two.height) return LandedHit.NEITHER;par

if (one.height == two.height) {
    // Successful block results in a counter against the attacker
    if (one.isAttack()) return LandedHit.PLAYER_TWO;
    return LandedHit.PLAYER_ONE;
}

En outre, le remplacement de certaines des ifinstructions par l'utilisation de l'opérateur ternaire ( boolean_expression ? result_if_true : result_if_false) pourrait rendre le code plus compact (par exemple, le code du bloc précédent deviendrait return one.isAttack() ? LandedHit.PLAYER_TWO : LandedHit.PLAYER_ONE;), mais cela peut conduire à des oneliners plus difficiles à lire, donc je ne le ferais pas '' Je le recommande pour des branchements plus complexes.

COUP
la source
Je vais certainement examiner cela, mais mon code actuel me permet d'utiliser la valeur int de oneet twod'être réutilisé comme points de départ sur ma feuille de sprites. Bien qu'il ne nécessite pas beaucoup de code supplémentaire pour permettre cela.
TomFirth
2
@ TomFirth84 Il y a une EnumMapclasse que vous pouvez utiliser pour cartographier les énumérations à vos décalages entiers (vous pouvez aussi utiliser les valeurs ordinales des membres enum directement, par exemple , Move.ATTACK_HIGH.ordinal()serait 0, Move.ATTACK_LOW.ordinal()serait 1, etc., mais qui est plus fragile / moins souple que explicitement associer chaque membre à une valeur en ajoutant des valeurs d'énumération entre les valeurs existantes détruirait le décompte, ce qui ne serait pas le cas avec un EnumMap.)
JAB
7
Il s'agit de la solution la plus lisible car elle traduit le code en quelque chose de significatif pour la personne qui lit le code.
David Stanley
Votre code, au moins celui qui utilise les énumérations, est incorrect. Selon les instructions if dans OP, un blocage réussi mène à un coup sûr sur l'attaquant. Mais +1 pour un code complet.
Taemyr
2
Vous pouvez même ajouter une attack(against)méthode à l' Moveénumération, renvoyant HIT lorsque le mouvement est une attaque réussie, BACKFIRE lorsque le mouvement est une attaque bloquée et RIEN quand ce n'est pas une attaque. De cette façon, vous pouvez l'implémenter en général ( public boolean attack(Move other) { if this.isAttack() return (other.isAttack() || other.height != this.height) ? HIT : BACKFIRE; return NOTHING; }), et le remplacer sur des mouvements spécifiques si nécessaire (mouvements faibles que tout bloc peut bloquer, attaques qui ne se retournent jamais, etc.)
réécrit
50

Pourquoi ne pas utiliser un tableau?

Je vais recommencer depuis le début. Je vois un modèle, les valeurs vont de 0 à 3 et vous voulez attraper toutes les valeurs possibles. Voici votre table:

0 & 0 = 0
0 & 1 = 0
0 & 2 = 1
0 & 3 = 2
1 & 0 = 0
1 & 1 = 0
1 & 2 = 2
1 & 3 = 1
2 & 0 = 2
2 & 1 = 1
2 & 2 = 3
2 & 3 = 3
3 & 0 = 2
3 & 1 = 1
3 & 2 = 3
3 & 3 = 3

lorsque nous regardons ce même tableau binaire, nous voyons les résultats suivants:

00 & 00 = 00
00 & 01 = 00
00 & 10 = 01
00 & 11 = 10
01 & 00 = 00
01 & 01 = 00
01 & 10 = 10
01 & 11 = 01
10 & 00 = 10
10 & 01 = 01
10 & 10 = 11
10 & 11 = 11
11 & 00 = 10
11 & 01 = 01
11 & 10 = 11
11 & 11 = 11

Maintenant, peut-être que vous voyez déjà un modèle, mais quand je combine la valeur un et deux, je vois que vous utilisez toutes les valeurs 0000, 0001, 0010, ..... 1110 et 1111. Maintenant, combinons la valeur un et deux pour en faire un seul Entier de 4 bits.

0000 = 00
0001 = 00
0010 = 01
0011 = 10
0100 = 00
0101 = 00
0110 = 10
0111 = 01
1000 = 10
1001 = 01
1010 = 11
1011 = 11
1100 = 10
1101 = 01
1110 = 11
1111 = 11

Lorsque nous traduisons cela en valeurs décimales, nous voyons un tableau très possible de valeurs où les un et deux combinés pourraient être utilisés comme index:

0 = 0
1 = 0
2 = 1
3 = 2
4 = 0
5 = 0
6 = 2
7 = 1
8 = 2
9 = 1
10 = 3
11 = 3
12 = 2
13 = 1
14 = 3
15 = 3

Le tableau est alors {0, 0, 1, 2, 0, 0, 2, 1, 2, 1, 3, 3, 2, 1, 3, 3}, où son index est simplement un et deux combinés.

Je ne suis pas un programmeur Java mais vous pouvez vous débarrasser de toutes les instructions if et l'écrire simplement comme ceci:

int[] myIntArray = {0, 0, 1, 2, 0, 0, 2, 1, 2, 1, 3, 3, 2, 1, 3, 3};
result = myIntArray[one * 4 + two]; 

Je ne sais pas si un décalage de bits par 2 est plus rapide que la multiplication. Mais cela pourrait valoir la peine d'essayer.

dj bazzie wazzie
la source
2
Un décalage de bit de 2 est presque certainement plus rapide qu'une multiplication de 4. Au mieux, la multiplication par 4 reconnaîtrait que 4 est 2 ^ 2 et effectue un décalage de bit lui-même (traduit potentiellement par le compilateur). Franchement, pour moi, le changement est plus lisible.
Cruncher
J'aime votre approche! Il aplatit essentiellement une matrice 4x4 en un tableau de 16 éléments.
Cameron Tinker
6
De nos jours, si je ne me trompe pas, le compilateur reconnaîtra sans aucun doute que vous multipliez par une puissance de deux et l'optimisera en conséquence. Donc pour vous, le programmeur, le décalage de bits et la multiplication devraient avoir exactement les mêmes performances.
Tanner Swett
24

Cela utilise un peu de bitmagic (vous le faites déjà en conservant deux bits d'informations (bas / haut et attaque / bloc) dans un seul entier):

Je ne l'ai pas exécuté, je l'ai seulement tapé ici, veuillez vérifier à nouveau. L'idée fonctionne sûrement. EDIT: Il est maintenant testé pour chaque entrée, fonctionne très bien.

public int fightMath(int one, int two) {
    if(one<2 && two<2){ //both players blocking
        return 0; // nobody hits
    }else if(one>1 && two>1){ //both players attacking
        return 3; // both hit
    }else{ // some of them attack, other one blocks
        int different_height = (one ^ two) & 1; // is 0 if they are both going for the same height - i.e. blocker wins, and 1 if height is different, thus attacker wins
        int attacker = one>1?1:0; // is 1 if one is the attacker, two is the blocker, and 0 if one is the blocker, two is the attacker
        return (attacker ^ different_height) + 1;
    }
}

Ou devrais-je suggérer de séparer les deux bits d'information en variables distinctes? Le code basé principalement sur des opérations binaires comme ci-dessus est généralement très difficile à maintenir.

elias
la source
2
Je suis d'accord avec cette solution, ressemble beaucoup à ce que j'avais à l'esprit dans mon commentaire sur la question principale. Je préférerais le diviser en variables séparées, ce qui faciliterait l'ajout d'une attaque intermédiaire par exemple à l'avenir.
Yoh
2
Je viens de corriger quelques bugs dans le code ci-dessus après l'avoir testé, maintenant cela fonctionne bien. Pour aller plus loin dans la manière du bit-manipulator, j'ai également trouvé une solution en une seule ligne, qui n'est toujours pas aussi mystique que le masque de bits dans d'autres réponses, mais qui est encore assez délicate pour vous faire perdre la tête:return ((one ^ two) & 2) == 0 ? (one & 2) / 2 * 3 : ((one & 2) / 2 ^ ((one ^ two) & 1)) + 1;
Elias
1
C'est la meilleure réponse car tout nouveau programmeur qui le lira comprendra réellement la magie qui se déroule derrière tous ces nombres magiques.
bezmax
20

Pour être honnête, chacun a son propre style de code. Je n'aurais pas pensé que les performances seraient trop affectées. Si vous comprenez mieux cela que d'utiliser une version de boîtier de commutation, continuez à utiliser cela.

Vous pouvez imbriquer les ifs, il y aurait donc potentiellement une légère augmentation des performances de vos dernières vérifications if, car elles n'auraient pas subi autant d'instructions if. Mais dans votre contexte d'un cours de base en Java, cela n'en bénéficiera probablement pas.

else if(one == 3 && two == 3) { result = 3; }

Donc, au lieu de ...

if(one == 0 && two == 0) { result = 0; }
else if(one == 0 && two == 1) { result = 0; }
else if(one == 0 && two == 2) { result = 1; }
else if(one == 0 && two == 3) { result = 2; }

Vous feriez ...

if(one == 0) 
{ 
    if(two == 0) { result = 0; }
    else if(two == 1) { result = 0; }
    else if(two == 2) { result = 1; }
    else if(two == 3) { result = 2; }
}

Et reformatez-le comme vous préférez.

Cela ne rend pas le code plus beau, mais l'accélère potentiellement un peu je crois.

Joe Harper
la source
3
Je ne sais pas si c'est vraiment une bonne pratique, mais dans ce cas, j'utiliserais probablement des instructions switch imbriquées. Cela prendrait plus de place mais ce serait vraiment clair.
teintures
Cela fonctionnerait aussi, mais je suppose que c'est une question de préférence. En fait, je préfère les instructions if, car cela indique pratiquement ce que fait le code. Ne démentez pas votre opinion bien sûr, tout ce qui vous convient :). Votez pour la suggestion alternative!
Joe Harper
12

Voyons ce que nous savons

1: vos réponses sont symétriques pour P1 (joueur un) et P2 (joueur deux). Cela a du sens pour un jeu de combat, mais c'est aussi quelque chose dont vous pouvez profiter pour améliorer votre logique.

2: 3 temps 0 temps 2 temps 1 temps 3. Les seuls cas non couverts par ces cas sont des combinaisons de 0 contre 1 et 2 contre 3. Pour le dire autrement, la table de victoire unique ressemble à ceci: 0 temps 2, 1 temps 3, 2 temps 1, 3 temps 0.

3: Si 0/1 s'affrontent, il y a égalité sans coup sûr, mais si 2/3 s'affrontent, les deux frappent

Tout d'abord, construisons une fonction à sens unique nous indiquant si nous avons gagné:

// returns whether we beat our opponent
public boolean doesBeat(int attacker, int defender) {
  int[] beats = {2, 3, 1, 0};
  return defender == beats[attacker];
}

Nous pouvons ensuite utiliser cette fonction pour composer le résultat final:

// returns the overall fight result
// bit 0 = one hits
// bit 1 = two hits
public int fightMath(int one, int two)
{
  // Check to see whether either has an outright winning combo
  if (doesBeat(one, two))
    return 1;

  if (doesBeat(two, one))
    return 2;

  // If both have 0/1 then its hitless draw but if both have 2/3 then they both hit.
  // We can check this by seeing whether the second bit is set and we need only check
  // one's value as combinations where they don't both have 0/1 or 2/3 have already
  // been dealt with 
  return (one & 2) ? 3 : 0;
}

Bien que cela soit sans doute plus complexe et probablement plus lent que la recherche de table proposée dans de nombreuses réponses, je pense que c'est une méthode supérieure car elle encapsule la logique de votre code et la décrit à quiconque lit votre code. Je pense que cela en fait une meilleure mise en œuvre.

(Cela fait un moment que je n'ai pas fait de Java, donc je m'excuse si la syntaxe est désactivée, j'espère qu'elle sera toujours intelligible si je me trompe un peu)

Soit dit en passant, 0-3 signifie clairement quelque chose; ce ne sont pas des valeurs arbitraires, il serait donc utile de les nommer.

Jack Aidley
la source
11

J'espère avoir bien compris la logique. Que diriez-vous de quelque chose comme:

public int fightMath (int one, int two)
{
    int oneHit = ((one == 3 && two != 1) || (one == 2 && two != 0)) ? 1 : 0;
    int twoHit = ((two == 3 && one != 1) || (two == 2 && one != 0)) ? 2 : 0;

    return oneHit+twoHit;
}

Vérifier un coup haut ou un coup bas n'est pas bloqué et il en va de même pour le joueur deux.

Edit: L'algorithme n'était pas entièrement compris, "hit" attribué lors du blocage que je n'avais pas réalisé (Thx elias):

public int fightMath (int one, int two)
{
    int oneAttack = ((one == 3 && two != 1) || (one == 2 && two != 0)) ? 1 : (one >= 2) ? 2 : 0;
    int twoAttack = ((two == 3 && one != 1) || (two == 2 && one != 0)) ? 2 : (two >= 2) ? 1 : 0;

    return oneAttack | twoAttack;
}
Chris
la source
Manière de trouver le grand résultat de bagoutage!
Chad
1
J'aime l'approche, mais je crains que cette solution n'ait complètement la possibilité de frapper en bloquant une attaque (par exemple, si un = 0 et deux = 2, il renvoie 0, mais 1 est attendu selon les spécifications). Peut-être que vous pouvez travailler dessus pour le faire correctement, mais je ne suis pas sûr que le code résultant serait toujours aussi élégant, car cela signifie que les lignes augmenteront un peu plus longtemps.
elias
Ne se rendait pas compte qu'un "coup" avait été accordé pour le blocage. Merci de l'avoir signalé. Ajusté avec une correction très simple.
Chris
10

Je n'ai pas d'expérience avec Java, donc il pourrait y avoir des fautes de frappe. Veuillez considérer le code comme un pseudo-code.

J'irais avec un simple interrupteur. Pour cela, vous auriez besoin d'une évaluation de numéro unique. Cependant, dans ce cas, depuis 0 <= one < 4 <= 9et 0 <= two < 4 <= 9, nous pouvons convertir les deux entiers en un simple int en multipliant onepar 10 et en ajoutant two. Ensuite, utilisez un commutateur dans le nombre résultant comme ceci:

public int fightMath(int one, int two) {
    // Convert one and two to a single variable in base 10
    int evaluate = one * 10 + two;

    switch(evaluate) {
        // I'd consider a comment in each line here and in the original code
        // for clarity
        case 0: result = 0; break;
        case 1: result = 0; break;
        case 1: result = 0; break;
        case 2: result = 1; break;
        case 3: result = 2; break;
        case 10: result = 0; break;
        case 11: result = 0; break;
        case 12: result = 2; break;
        case 13: result = 1; break;
        case 20: result = 2; break;
        case 21: result = 1; break;
        case 22: result = 3; break;
        case 23: result = 3; break;
        case 30: result = 1; break;
        case 31: result = 2; break;
        case 32: result = 3; break;
        case 33: result = 3; break;
    }

    return result;
}

Il y a une autre méthode courte que je veux juste souligner comme un code théorique. Cependant, je ne l'utiliserais pas car il a une complexité supplémentaire que vous ne voulez normalement pas traiter. La complexité supplémentaire vient de la base 4 , car le comptage est 0, 1, 2, 3, 10, 11, 12, 13, 20, ...

public int fightMath(int one, int two) {
    // Convert one and two to a single variable in base 4
    int evaluate = one * 4 + two;

    allresults = new int[] { 0, 0, 1, 2, 0, 0, 2, 1, 2, 1, 3, 3, 1, 2, 3, 3 };

    return allresults[evaluate];
}

Vraiment juste une note supplémentaire, au cas où je manquerais quelque chose de Java. En PHP, je ferais:

function fightMath($one, $two) {
    // Convert one and two to a single variable in base 4
    $evaluate = $one * 10 + $two;

    $allresults = array(
         0 => 0,  1 => 0,  2 => 1,  3 => 2,
        10 => 0, 11 => 0, 12 => 2, 13 => 1,
        20 => 2, 21 => 1, 22 => 3, 23 => 3,
        30 => 1, 31 => 2, 32 => 3, 33 => 3 );

    return $allresults[$evaluate];
}
Francisco Presencia
la source
Java n'a pas de lamdas avant la 8e version
Kirill Gamazkov
1
Ce. Pour un si petit nombre d'entrées, j'utiliserais un commutateur avec une valeur composite (bien qu'il puisse être plus lisible avec un multiplicateur supérieur à 10, comme 100 ou 1000).
Medinoc
7

Puisque vous préférez les ifconditions imbriquées , voici une autre façon.
Notez qu'il n'utilise pas le resultmembre et qu'il ne change aucun état.

public int fightMath(int one, int two) {
    if (one == 0) {
      if (two == 0) { return 0; }
      if (two == 1) { return 0; }
      if (two == 2) { return 1; }
      if (two == 3) { return 2; }
    }   
    if (one == 1) {
      if (two == 0) { return 0; }
      if (two == 1) { return 0; }
      if (two == 2) { return 2; }
      if (two == 3) { return 1; }
    }
    if (one == 2) {
      if (two == 0) { return 2; }
      if (two == 1) { return 1; }
      if (two == 2) { return 3; }
      if (two == 3) { return 3; }
    }
    if (one == 3) {
      if (two == 0) { return 1; }
      if (two == 1) { return 2; }
      if (two == 2) { return 3; }
      if (two == 3) { return 3; }
    }
    return DEFAULT_RESULT;
}
Nick Dandoulakis
la source
pourquoi n'en avez-vous pas d'autres?
FDinoff
3
@FDinoff J'aurais pu utiliser des elsechaînes mais cela n'avait fait aucune différence.
Nick Dandoulakis
1
Je sais que c'est trivial, mais l'ajout des chaînes else ne s'exécuterait-il pas plus rapidement? dans 3 cas sur 4? Je prends toujours l'habitude d'écrire du code pour l'exécuter le plus rapidement possible même si ce n'est que quelques cycles.
Brandon Bearden
2
@BrandonBearden ici, ils ne feront aucune différence (en supposant que l'entrée est toujours dans la plage 0..3). Le compilateur va probablement micro-optimiser le code de toute façon. Si nous avons une longue série d' else ifinstructions, nous pouvons accélérer le code avec switchou rechercher des tables.
Nick Dandoulakis
Comment en est-il ainsi? S'il one==0exécute le code, il devra alors vérifier si one==1puis si one==2et enfin si one==3- S'il y avait d'autres if là-dedans, il ne ferait pas les trois dernières vérifications car il quitterait l'instruction après la première correspondance. Et oui, vous pouvez encore optimiser en utilisant une instruction switch à la place des if (one...instructions, puis en utilisant un autre interrupteur à l'intérieur du cas de la one's. Mais ce n'est pas ma question.
Brandon Bearden
6

Essayez-le avec un boîtier d'interrupteur. ..

Jetez un œil ici ou ici pour plus d'informations à ce sujet

switch (expression)
{ 
  case constant:
        statements;
        break;
  [ case constant-2:
        statements;
        break;  ] ...
  [ default:
        statements;
        break;  ] ...
}

Vous pouvez y ajouter plusieurs conditions (pas simultanément) et même avoir une option par défaut où aucun autre cas n'a été satisfait.

PS: Seulement si une condition doit être remplie ..

Si 2 conditions surviennent simultanément .. Je ne pense pas que l'interrupteur puisse être utilisé. Mais vous pouvez réduire votre code ici.

Instruction de commutateur Java plusieurs cas

Nevin Madhukar K
la source
6

La première chose qui m'est venue à l'esprit était essentiellement la même réponse donnée par Francisco Presencia, mais quelque peu optimisée:

public int fightMath(int one, int two)
{
    switch (one*10 + two)
    {
    case  0:
    case  1:
    case 10:
    case 11:
        return 0;
    case  2:
    case 13:
    case 21:
    case 30:
        return 1;
    case  3:
    case 12:
    case 20:
    case 31:
        return 2;
    case 22:
    case 23:
    case 32:
    case 33:
        return 3;
    }
}

Vous pouvez l'optimiser davantage en faisant du dernier cas (pour 3) le cas par défaut:

    //case 22:
    //case 23:
    //case 32:
    //case 33:
    default:
        return 3;

L'avantage de cette méthode est qu'il est plus facile de voir quelles valeurs pour oneet twocorrespondent à quelles valeurs de retour que certaines des autres méthodes suggérées.

David R Tribble
la source
Ceci est une variation d'une autre réponse de la mienne ici .
David R Tribble
6
((two&2)*(1+((one^two)&1))+(one&2)*(2-((one^two)&1)))/2
Dawood ibn Kareem
la source
4
Combien de temps vous at-il fallu pour en arriver là?
mbatchkarov
2
@mbatchkarov Environ 10 minutes de lecture des autres réponses, puis 10 minutes de gribouillage au crayon et au papier.
Dawood ibn Kareem
7
Je serais vraiment triste si je devais maintenir cela.
Meryovi
euhmmm ... Il y a un bug: vous manquez; --unicorns
Alberto
Je suis d'accord avec @Meryovi, des accessoires pour être concis, mais horrible comme le code APL
Ed Griebel
3

Alors que je dessine un tableau entre un / deux et le résultat, je vois un motif,

if(one<2 && two <2) result=0; return;

Ce qui précède réduirait au moins 3 si les déclarations. Je ne vois pas de modèle d'ensemble ni je ne peux glaner beaucoup du code donné - mais si une telle logique peut être dérivée, cela réduirait un certain nombre d'instructions if.

J'espère que cela t'aides.

AnonNihcas
la source
3

Un bon point serait de définir les règles comme du texte, vous pouvez alors plus facilement dériver la bonne formule. Ceci est extrait de la belle représentation de tableau de laalto:

{ 0, 0, 1, 2 },
{ 0, 0, 2, 1 },
{ 2, 1, 3, 3 },
{ 1, 2, 3, 3 }

Et nous allons ici avec quelques commentaires généraux, mais vous devez les décrire en termes de règles:

if(one<2) // left half
{
    if(two<2) // upper left half
    {
        result = 0; //neither hits
    }
    else // lower left half
    {
        result = 1+(one+two)%2; //p2 hits if sum is even
    }
}
else // right half
{
    if(two<2) // upper right half
    {
        result = 1+(one+two+1)%2; //p1 hits if sum is even
    }
    else // lower right half
    {
        return 3; //both hit
    }
}

Vous pouvez bien sûr réduire cela à moins de code, mais c'est généralement une bonne idée de comprendre ce que vous codez plutôt que de trouver une solution compacte.

if((one<2)&&(two<2)) result = 0; //top left
else if((one>1)&&(two>1)) result = 3; //bottom right
else result = 1+(one+two+((one>1)?1:0))%2; //no idea what that means

Une explication sur les hits p1 / p2 compliqués serait super, ça a l'air intéressant!

Marcellus
la source
3

La solution la plus courte et toujours lisible:

static public int fightMath(int one, int two)
{
    if (one < 2 && two < 2) return 0;
    if (one > 1 && two > 1) return 3;
    int n = (one + two) % 2;
    return one < two ? 1 + n : 2 - n;
}

ou encore plus court:

static public int fightMath(int one, int two)
{
    if (one / 2 == two / 2) return (one / 2) * 3;
    return 1 + (one + two + one / 2) % 2;
}

Ne contient aucun chiffre "magique";) J'espère que ça aide.

PW
la source
2
De telles formules rendront impossible de modifier (mettre à jour) le résultat d'une combinaison à une date ultérieure. La seule façon serait de retravailler la formule entière.
SNag
2
@SNag: Je suis d'accord avec ça. La solution la plus flexible consiste à utiliser le réseau 2D. Mais l'auteur de ce post voulait une formule et c'est la meilleure que vous puissiez obtenir avec seulement des ifs et des mathématiques simples.
PW
2

static int val(int i, int u){ int q = (i & 1) ^ (u & 1); return ((i >> 1) << (1 ^ q))|((u >> 1) << q); }

user1837841
la source
1

J'aime personnellement mettre en cascade les opérateurs ternaires:

int result = condition1
    ? result1
    : condition2
    ? result2
    : condition3
    ? result3
    : resultElse;

Mais dans votre cas, vous pouvez utiliser:

final int[] result = new int[/*16*/] {
    0, 0, 1, 2,
    0, 0, 2, 1,
    2, 1, 3, 3,
    1, 2, 3, 3
};

public int fightMath(int one, int two) {
    return result[one*4 + two];
}

Ou, vous pouvez remarquer un modèle en bits:

one   two   result

section 1: higher bits are equals =>
both result bits are equals to that higher bits

00    00    00
00    01    00
01    00    00
01    01    00
10    10    11
10    11    11
11    10    11
11    11    11

section 2: higher bits are different =>
lower result bit is inverse of lower bit of 'two'
higher result bit is lower bit of 'two'

00    10    01
00    11    10
01    10    10
01    11    01
10    00    10
10    01    01
11    00    01
11    01    10

Vous pouvez donc utiliser la magie:

int fightMath(int one, int two) {
    int b1 = one & 2, b2 = two & 2;
    if (b1 == b2)
        return b1 | (b1 >> 1);

    b1 = two & 1;

    return (b1 << 1) | (~b1);
}
Kirill Gamazkov
la source
1

Voici une version assez concise, similaire à la réponse de JAB . Cela utilise une carte pour stocker ce qui triomphe des autres.

public enum Result {
  P1Win, P2Win, BothWin, NeitherWin;
}

public enum Move {
  BLOCK_HIGH, BLOCK_LOW, ATTACK_HIGH, ATTACK_LOW;

  static final Map<Move, List<Move>> beats = new EnumMap<Move, List<Move>>(
      Move.class);

  static {
    beats.put(BLOCK_HIGH, new ArrayList<Move>());
    beats.put(BLOCK_LOW, new ArrayList<Move>());
    beats.put(ATTACK_HIGH, Arrays.asList(ATTACK_LOW, BLOCK_LOW));
    beats.put(ATTACK_LOW, Arrays.asList(ATTACK_HIGH, BLOCK_HIGH));
  }

  public static Result compare(Move p1Move, Move p2Move) {
    boolean p1Wins = beats.get(p1Move).contains(p2Move);
    boolean p2Wins = beats.get(p2Move).contains(p1Move);

    if (p1Wins) {
      return (p2Wins) ? Result.BothWin : Result.P1Win;
    }
    if (p2Wins) {
      return (p1Wins) ? Result.BothWin : Result.P2Win;
    }

    return Result.NeitherWin;
  }
} 

Exemple:

System.out.println(Move.compare(Move.ATTACK_HIGH, Move.BLOCK_LOW));

Tirages:

P1Win
Duncan Jones
la source
Je recommanderais static final Map<Move, List<Move>> beats = new java.util.EnumMap<>();plutôt, il devrait être légèrement plus efficace.
JAB
@JAB Oui, bonne idée. J'oublie toujours que ce type existe. Et ... comme c'est difficile d'en construire un!
Duncan Jones
1

J'utiliserais une carte, un HashMap ou un TreeMap

Surtout si les paramètres ne sont pas sur le formulaire 0 <= X < N

Comme un ensemble d'entiers positifs aléatoires ..

Code

public class MyMap
{
    private TreeMap<String,Integer> map;

    public MyMap ()
    {
        map = new TreeMap<String,Integer> ();
    }

    public void put (int key1, int key2, Integer value)
    {
        String key = (key1+":"+key2);

        map.put(key, new Integer(value));
    }

    public Integer get (int key1, int key2)
    {
        String key = (key1+":"+key2);

        return map.get(key);
    }
}
Khaled.K
la source
1

Merci à @Joe Harper car j'ai fini par utiliser une variante de sa réponse. Pour le réduire davantage, 2 résultats pour 4 étant les mêmes, je l'ai encore réduit.

Je pourrais y revenir à un moment donné, mais s'il n'y a pas de résistance majeure causée par plusieurs ifdéclarations, je vais garder cela pour l'instant. J'examinerai la matrice de table et les solutions de déclaration de commutateur plus loin.

public int fightMath(int one, int two) {
  if (one === 0) {
    if (two === 2) { return 1; }
    else if(two === 3) { return 2; }
    else { return 0; }
  } else if (one === 1) {
    if (two === 2) { return 2; }
    else if (two === 3) { return 1; }
    else { return 0; }
  } else if (one === 2) {
    if (two === 0) { return 2; }
    else if (two === 1) { return 1; }
    else { return 3; }
  } else if (one === 3) {
    if (two === 0) { return 1; }
    else if (two === 1) { return 2; }
    else { return 3; }
  }
}
TomFirth
la source
13
Ceci est en fait moins lisible que l'original et ne réduit pas le nombre de déclarations if ...
Chad
@Chad L'idée était d'augmenter la vitesse du processus et bien que cela semble horrible, il est facilement mis à jour si j'ajoute plus d'actions à l'avenir. En disant cela, j'utilise maintenant une réponse précédente que je n'ai pas bien comprise auparavant.
TomFirth
3
@ TomFirth84 Y a-t-il une raison pour laquelle vous ne respectez pas les conventions de codage appropriées pour vos instructions if?
ylun.ca
@ylun: J'avais réduit les lignes avant de les coller sur SO, non pas pour la lisibilité mais pour l'espace de spam. Il y a des variantes de pratique sur cette page et malheureusement c'est juste la façon dont j'ai appris et avec laquelle je suis à l'aise.
TomFirth
2
@ TomFirth84 Je ne pense pas que cela soit facile à mettre à jour, le nombre de lignes croît quelque chose comme le produit du nombre de valeurs autorisées.
Andrew Lazarus
0
  1. Utilisez des constantes ou des énumérations pour rendre le code plus lisible
  2. Essayez de diviser le code en plus de fonctions
  3. Essayez d'utiliser la symétrie du problème

Voici une suggestion à quoi cela pourrait ressembler, mais l'utilisation d'un ints ici est toujours un peu moche:

static final int BLOCK_HIGH = 0;
static final int BLOCK_LOW = 1;
static final int ATTACK_HIGH = 2;
static final int ATTACK_LOW = 3;

public static int fightMath(int one, int two) {
    boolean player1Wins = handleAttack(one, two);
    boolean player2Wins = handleAttack(two, one);
    return encodeResult(player1Wins, player2Wins); 
}



private static boolean handleAttack(int one, int two) {
     return one == ATTACK_HIGH && two != BLOCK_HIGH
        || one == ATTACK_LOW && two != BLOCK_LOW
        || one == BLOCK_HIGH && two == ATTACK_HIGH
        || one == BLOCK_LOW && two == ATTACK_LOW;

}

private static int encodeResult(boolean player1Wins, boolean player2Wins) {
    return (player1Wins ? 1 : 0) + (player2Wins ? 2 : 0);
}

Il serait préférable d'utiliser un type structuré pour l'entrée et la sortie. L'entrée a en fait deux champs: la position et le type (bloc ou attaque). La sortie a également deux champs: player1Wins et player2Wins. L'encodage en un seul entier rend la lecture du code plus difficile.

class PlayerMove {
    PlayerMovePosition pos;
    PlayerMoveType type;
}

enum PlayerMovePosition {
    HIGH,LOW
}

enum PlayerMoveType {
    BLOCK,ATTACK
}

class AttackResult {
    boolean player1Wins;
    boolean player2Wins;

    public AttackResult(boolean player1Wins, boolean player2Wins) {
        this.player1Wins = player1Wins;
        this.player2Wins = player2Wins;
    }
}

AttackResult fightMath(PlayerMove a, PlayerMove b) {
    return new AttackResult(isWinningMove(a, b), isWinningMove(b, a));
}

boolean isWinningMove(PlayerMove a, PlayerMove b) {
    return a.type == PlayerMoveType.ATTACK && !successfulBlock(b, a)
            || successfulBlock(a, b);
}

boolean successfulBlock(PlayerMove a, PlayerMove b) {
    return a.type == PlayerMoveType.BLOCK 
            && b.type == PlayerMoveType.ATTACK 
            && a.pos == b.pos;
}

Malheureusement, Java n'est pas très bon pour exprimer ces types de types de données.

peq
la source
-2

Faites plutôt quelque chose comme ça

   public int fightMath(int one, int two) {
    return Calculate(one,two)

    }


    private int Calculate(int one,int two){

    if (one==0){
        if(two==0){
     //return value}
    }else if (one==1){
   // return value as per condtiion
    }

    }
onkar
la source
4
Vous venez de créer une fonction privée enveloppée par la fonction publique. Pourquoi ne pas simplement mettre cela en œuvre dans la fonction publique?
Martin Ueding
5
Et vous n'avez pas réduit le nombre d'instructions if.
Chad