Trop de déclarations “if”?

Le code suivant fonctionne comme je le souhaite, mais c’est moche, excessif ou un certain nombre d’autres choses. J’ai examiné des formules et essayé d’écrire quelques solutions, mais je me retrouve avec un nombre similaire de déclarations.

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

Pour expliquer le code, c’est pour une sorte de jeu simultané au tour par tour. 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 atsortingbué si cela aide. Le résultat est 0 = ni 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; } 

Si vous ne pouvez pas proposer de formule, vous pouvez utiliser un tableau pour un nombre 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]; 

Comme votre jeu de données est si petit, vous pouvez tout compresser en un entier long et le transformer en une formule

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

Variante plus binary:

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 l'OP est une carte de 2 nombres (un, deux), domaine {0,1,2,3} à l'intervalle {0,1,2,3}. Chacune des réponses a abordé la manière 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 4 à 4 chiffres N (un, deux) où un est le chiffre 1, deux 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 numéro de base 4 à 1 chiffre peut être exprimé sous la forme d'un nombre 2 de base à 2 chiffres. {0,1,2,3} = {00,01,10,11}, et donc chaque sortie peut être codée avec seulement 2 bits. De là-haut, il n'y a que 16 sorties différentes possibles, donc il suffit de 16 * 2 = 32 bits pour encoder la carte entière; Cela peut tout faire 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 rest plus qu'à indexer et à retourner la partie droite de la constante. Dans ce cas, vous pouvez décaler M à droite 2 * N fois et prendre les 2 bits les moins significatifs, à savoir (M >> 2 * N) & 0x3. Les expressions (un < < 3) et (deux << 1) ne font que multiplier les choses en notant que 2 * x = x << 1 et 8 * x = x << 3.

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 de ce qui est calculé ici: cela souligne que nous calculons qui est touché par quelle attaque, et que nous 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 la sémantique du jeu soit claire. C’est-à-dire que plutôt que “une attaque de zéro et une défense contre un résultat”, trouvez plutôt un moyen de faire en sorte que le code implique plus clairement “une attaque de coup bas et une défense de bloc faible sans coup”. Faites en sorte que le code reflète la logique métier du jeu.

Vous pouvez créer une masortingce 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 allez utiliser

 public int fightMath(int one, int two) { return this.results[one][two]; } 

D’autres personnes ont déjà suggéré mon idée initiale, la méthode masortingx, 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 se situent dans la plage attendue et en utilisant des retours sur place. les normes que j’ai vues imposent un sharepoint sortie pour les fonctions, mais j’ai constaté que les retours multiples sont très utiles pour éviter le codage des flèches et avec la prévalence des exceptions en Java, il est inutile d’appliquer une telle règle comme toute exception non capturée jetée à l’intérieur de la méthode est un sharepoint sortie possible de toute façon). L’imbrication des instructions de commutation est une possibilité, mais pour le petit nombre de valeurs que vous vérifiez ici, je trouve que les instructions sont plus compactes et ne risquent pas de générer beaucoup de différences de performances, en particulier si votre programme est basé sur le tour plutôt que réel -temps.

 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 que cela pourrait être dû à l’irrégularité de certaines parties du mappage d’entrée-> résultat. Je privilégie plutôt le style masortingciel en raison de sa simplicité et de la manière dont vous pouvez configurer la masortingce pour donner un sens visuel (même si cela est en partie influencé par mes souvenirs des cartes 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 par propriété / atsortingbut pour les entrées et le résultat et modifie également le résultat pour tenir compte du blocage, ce qui devrait fonction lisible.

 enum MoveType { ATTACK, BLOCK; } enum MoveHeight { HIGH, LOW; } enum Move { // Enum members can have properties/atsortingbutes/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 append des blocs / attaques de plus hauteurs, juste les énumérations; L’ajout de types de déplacements supplémentaires nécessitera probablement une modification de la fonction. EnumSet peut également être plus extensible que l’utilisation d’énumérations supplémentaires comme propriétés de l’énumération principale, par exemple EnumSet attacks = EnumSet.of(Move.ATTACK_HIGH, Move.ATTACK_LOW, ...); et ensuite des attacks.contains(move) plutôt que move.type == MoveType.ATTACK , bien que l’utilisation d’ EnumSet s soit probablement légèrement plus lente que les vérifications égales directes.


if (one.height == two.height) return LandedHit.NEITHER; un bloc aboutit à un compteur, vous pouvez remplacer if (one.height == two.height) return LandedHit.NEITHER; avec

 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, remplacer certaines des instructions if 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 return one.isAttack() ? LandedHit.PLAYER_TWO : LandedHit.PLAYER_ONE; plus difficiles à lire, donc je ne le recommanderais pas pour des ramifications plus complexes.

Pourquoi ne pas utiliser un tableau?

Je vais commencer depuis le début. Je vois un motif, 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 

quand on regarde cette même table binary on voit 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 motif, mais quand je combine les valeurs 1 et 2, je vois que vous utilisez toutes les valeurs 0000, 0001, 0010, ….. 1110 et 1111. Maintenant, combinons les valeurs 1 et 2 pour créer un seul Entier 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 ceci en valeurs décimales, nous voyons un tableau très possible de valeurs où les 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} , son index étant 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 simplement les écrire 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 2 bits est plus rapide que la multiplication. Mais cela pourrait valoir la peine d’essayer.

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

Je ne l’ai pas couru, je l’ai seulement tapé ici, s’il vous plaît double-cliquez. L’idée fonctionne sûrement. EDIT: Il est maintenant testé pour chaque entrée, fonctionne 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 - ie 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 les opérations sur bits comme ci-dessus est généralement très difficile à maintenir.

Pour être tout à fait honnête, tout le monde a son propre style de code. Je n’aurais pas pensé que la performance serait trop affectée. Si vous comprenez mieux que d’utiliser une version à boîtier de commutation, continuez à l’utiliser.

Vous pourriez imbriquer les ifs, donc il pourrait y avoir une légère augmentation de performance pour vos dernières vérifications, car elles n’auraient pas passé autant d’instructions if. Mais dans votre contexte de cours java de base, cela ne profitera 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; } 

Tu le ferais …

 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 le souhaitez.

Cela ne rend pas le code meilleur, mais le fait accélérer un peu, je crois.

Voyons ce que nous soaps

1: vos réponses sont symésortingques pour P1 (joueur un) et P2 (joueur deux). Cela est logique pour un jeu de combat, mais vous pouvez également en tirer parti pour améliorer votre logique.

2: 3 battements 0 battements 2 battements 1 battements 3. Les seuls cas non couverts par ces cas sont des combinaisons de 0 contre 1 et de 2 contre 3. En d’autres termes, la table de victoire unique ressemble à ceci: 0 temps 2, 1 temps 3, 2 temps 1, 3 temps 0.

3: Si 0/1 monte les uns contre les autres, il y a un tirage sans tacle mais si les 2/3 montent contre chacun des deux, ils 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 alors 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 ousortingght 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 consultation 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 implémentation.

(Il y a longtemps que je n’ai pas fait de Java, je m’excuse si la syntaxe est désactivée, j’espère qu’elle est encore intelligible si je me trompe un peu)

Au fait, 0-3 signifie clairement quelque chose; ce ne sont pas des valeurs arbitraires, alors ça aiderait à les nommer.

J’espère que je comprends la logique correctement. 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 le même pour le joueur deux.

Edit: L’algorithme n’a pas été entièrement compris, “hit” atsortingbué lors du blocage que je n’ai 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; } 

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

J’irais avec un simple commutateur. Pour cela, il vous faudrait une évaluation unique. Cependant, pour ce cas, puisque 0 < = one < 4 <= 9 et 0 < = two < 4 <= 9 , nous pouvons convertir les deux ints en un simple int en multipliant one par 10 et en ajoutant two . Ensuite, utilisez un commutateur dans le numéro 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 en tant que code théorique. Cependant, je ne l'utiliserais pas parce que cela ne présente pas la complexité supplémentaire que vous ne voulez normalement pas gérer. 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]; } 

Puisque vous préférez nested if conditionnel, voici une autre façon.
Notez qu’il n’utilise pas le membre result 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; } 

Essayez-le avec un boîtier de commutateur. ..

Jetez un oeil 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 append plusieurs conditions (pas simultanément) et même une option par défaut où aucun autre cas n’a été satisfait.

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

Si 2 conditions se présentent simultanément, je ne pense pas que le commutateur puisse être utilisé. Mais vous pouvez réduire votre code ici.

Déclaration de commutateur Java plusieurs cas

La première chose qui m’est apparue était essentiellement la même réponse donnée par Francisco Presencia, mais optimisée quelque peu:

 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 one et two correspondent à des valeurs de retour que certaines des autres méthodes suggérées.

 ((two&2)*(1+((one^two)&1))+(one&2)*(2-((one^two)&1)))/2 

You may use a switch case instead of mutiple if

Also to mention that since you have two variables then you have to merge the two variables to use them in switch

Check this Java switch statement to handle two variables?

As I draw a table between one/two and the result, I see one pattern,

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

The above would cut down atleast 3 if statements. I don't see a set pattern nor I am able to glean much from the code given - but if such logic can be derived, it would cut down a number of if statements.

J'espère que cela t'aides.

A good point would be to define the rules as text, you can easier derive the correct formula then. This is extracted from laalto’s nice array representation:

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

And here we go with some general comments, but you should describe them in rule terms:

 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 } } 

You could of course crunch this down to less code, but it is generally a good idea to understand what you code rather than finding a compact solution.

 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 

Some explanation on the complicated p1/p2 hits would be great, looks interesting!

The shortest and still readable solution:

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

or even shorter:

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

Doesn't contain any "magic" numbers 😉 Hope it helps.

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

I personally like to cascade ternary operators:

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

But in your case, you can use:

 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]; } 

Or, you can notice a pattern in 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 

So you can use magic:

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

Here’s a fairly concise version, similar to JAB’s response . This utilises a map to store which moves sortingumph over others.

 public enum Result { P1Win, P2Win, BothWin, NeitherWin; } public enum Move { BLOCK_HIGH, BLOCK_LOW, ATTACK_HIGH, ATTACK_LOW; static final Map> beats = new EnumMap>( Move.class); static { beats.put(BLOCK_HIGH, new ArrayList()); beats.put(BLOCK_LOW, new ArrayList()); 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)); 

Impressions:

 P1Win 

I’d use a Map, either a HashMap or a TreeMap

Especially if the parameters are not on the form 0 < = X < N

Like a set of random positive integers ..

Code

 public class MyMap { private TreeMap map; public MyMap () { map = new TreeMap (); } public void put (int key1, int key2, Integer value) { Ssortingng key = (key1+":"+key2); map.put(key, new Integer(value)); } public Integer get (int key1, int key2) { Ssortingng key = (key1+":"+key2); return map.get(key); } } 

Thanks to @Joe Harper as I ended up using a variation of his answer. To slim it down further as 2 results per 4 were the same I slimmed it down further.

I may come back to this at some point, but if there’s no major resistance caused by multiple if -statements then I’ll keep this for now. I will look into the table masortingx and switch statement solutions further.

 public int fightMath(int one, int two) { if (one == 0) { if (two == 2) { result = 1; } else if(two == 3) { result = 2; } else { result = 0; } } else if(one == 1) { if (two == 2) { result = 2; } else if(two == 3) { result = 1; } else { result = 0; } } else if(one == 2) { if (two == 0) { result = 2; } else if(two == 1) { result = 1; } else { result = 3; } } else if(one == 3) { if (two == 0) { result = 1; } else if(two == 1) { result = 2; } else { result = 3; } } return result; } 
  1. Use constants or enums to make the code more readable
  2. Try to split the code into more functions
  3. Try to use the symmetry of the problem

Here is a suggestion how this could look like, but using an ints here is still kind of ugly:

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

It would be nicer to use a structured type for the input and the output. The input actually has two fields: the position and the type (block or attack). The output also has two fields: player1Wins and player2Wins. Encoding this into a single integer makes it harder to read the code.

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

Unfortunately, Java is not very good at expressing those kinds of data-types.

Instead do something like this

  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 } }