太多“if”陈述？
下面的代码确实工作，我需要它，但它是丑陋，过度或其他一些事情。 我已经看过公式并试图写出一些解决scheme，但是我最终得到了类似的陈述。
在这种情况下是否有一种math公式可以使我受益？
为了解释代码，这是一种基于同时回合的游戏..两个玩家每个都有四个动作button，结果来自一个数组（0-3），但是variables'one'＆'two'可以是分配任何东西，如果有帮助 结果是，0 =不赢，1 = p1胜，2 = p2胜，3 =双赢。
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; } 如果你不能拿出一个公式，你可以使用一个表来获得如此有限的结果：
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];
由于您的数据集非常小，您可以将所有内容压缩为1个长整数并将其转换为公式
public int fightMath(int one,int two) { return (int)(0xF9F66090L >> (2*(one*4 + two)))%4; }
更多按位变体：
这使事实上所有事情都是2的倍数
public int fightMath(int one,int two) { return (0xF9F66090 >> ((one << 3) | (two << 1))) & 0x3; }
魔术常数的起源
我能说什么？ 世界需要魔法，有时可能有某种东西要求创造。
解决OP问题的函数的本质是从2个数字（1，2），域{0,1,2,3}到范围{0,1,2,3}的映射。 每个答案已经接近如何实现该地图。
另外，你可以在一些答案中看到一个重复的问题，如一个2位数的基数4的数字N（1,2），其中一个是数字1，两个是数字2，N = 4 * 1 +两个; N = {0,1,2，…，15} – 十六个不同的值，这很重要。 该函数的输出是一个1位数的基数4个数{0,1,2,3} – 4个不同的数值，也很重要。
现在，1位数的基数4号码可以表示为2位数的基数2号码; {0,1,2,3} = {00,01,10,11}，所以每个输出只能用2位编码。 从上面看，只有16个不同的输出可能，所以16 * 2 = 32比特是编码整个地图所需的全部; 这可以全部适合1个整数。
常数M是m（0）以M [0：1]位编码，m（1）以M [2：3]位编码的映射m的编码，m（n）以位M [N * 2：N * 2 + 1]。
剩下的就是索引和返回常量的右边部分，在这种情况下，可以将M右移2 * N次，并取2个最低有效位，即（M >> 2 * N）＆0x3。 expression式（one << 3）和（two << 1）只是将事物相乘，同时注意到2 * x = x << 1和8 * x = x << 3。
我不喜欢除JAB之外的任何解决scheme。 其他人都不容易阅读代码并理解正在计算的内容 。
下面是我将如何编写这段代码 – 我只知道C＃，而不是Java，但是你得到的图片：
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); }
现在更清楚的是这里计算的是什么：这强调我们计算谁受到什么攻击，并返回两个结果。
然而，这可能会更好; 那个布尔数组有点不透明。 我喜欢查表方式，但是我倾向于用这种方式来写，这样就明确了游戏的语义是什么。 也就是说，不是“零攻击，一击不攻击”，而是想方设法让代码更加清晰，意味着“低冲击，低防守不成功”。 使代码反映游戏的商业逻辑。
您可以创build包含结果的matrix
int[][] results = {{0, 0, 1, 2}, {0, 0, 2, 1},{2, 1, 3, 3},{2, 1, 3, 3}};
当你想获得价值，你会使用
public int fightMath(int one, int two) { return this.results[one][two]; }
其他人已经提出了我最初的想法，即matrix方法，但除了巩固if语句之外，还可以通过确保提供的参数处于预期的范围内并使用就地返回来避免某些内容（某些编码标准我已经看到强制实现函数的一个退出点，但是我发现多个返回对于避免箭头编码是非常有用的，而且在Java中的例外情况普遍存在，所以没有太多的意义在严格执行这样的规则因为在方法内抛出的任何未捕获的exception都是可能的退出点）。 嵌套开关语句是一种可能性，但对于你在这里检查的小范围的值，我发现如果语句更紧凑，并且不太可能导致性能差别很大，特别是如果你的程序是基于回合的而不是真实的-时间。
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 }
这最终会导致可读性低于否则可能是由于input – >结果映射部分的不规则性。 我赞成matrix的风格，而不是由于它的简单性，以及如何设置matrix使其在视觉上有意义（尽pipe这部分受到我对卡诺图的记忆的影响）：
int[][] results = {{0, 0, 1, 2}, {0, 0, 2, 1}, {2, 1, 3, 3}, {2, 1, 3, 3}};
更新：鉴于你提到的阻塞/击中，这里是一个更激进的变化，利用属性/属性持有枚举types的input和结果，也修改了一点结果考虑阻止，这应该导致更多可读function。
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; }
如果你想添加更多高度的块/攻击，你甚至不需要改变函数本身，只需要枚举; 不过，添加其他types的移动可能需要修改该function。 此外， EnumSet可能比使用额外的枚举作为主枚举的属性更具扩展性，例如EnumSet<Move> attacks = EnumSet.of(Move.ATTACK_HIGH, Move.ATTACK_LOW, ...); 然后move.type == MoveType.ATTACK attacks.contains(move)而不是move.type == MoveType.ATTACK ，虽然使用EnumSet s可能会比直接equals检查稍慢。
对于成功的块产生计数器的情况，可以replaceif (one.height == two.height) return LandedHit.NEITHER; 同
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; }
另外，使用三元运算符（ boolean_expression ? result_if_true : result_if_false ）replace一些if语句可以使代码更加紧凑（例如，前面块中的代码将return one.isAttack() ? LandedHit.PLAYER_TWO : LandedHit.PLAYER_ONE; ），但是这会导致难以阅读的线索，所以我不会推荐它用于更复杂的分支。
为什么不使用数组？
我将从头开始。 我看到一个模式，值从0到3，你想抓住所有可能的值。 这是你的桌子：
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
当我们查看这个相同的表格二进制文件时，我们看到以下结果：
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
现在，也许你已经看到了一些模式，但是当我把数值1和数字2相结合时，我发现你正在使用所有的值0000，0001，0010，….. 1110和1111.现在，让我们将值1和2合并为一个4位整数。
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
当我们将其转换回十进制值时，我们看到一个非常可能的数值数组，其中一个和两个组合可以用作索引：
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
这个数组是{0,0,1,2,0,0,2,1,2,1,3,3,2,1,3,3}，它的索引只是一个和两个组合。
我不是一个Java程序员，但你可以摆脱所有的if语句，只是写下来，像这样：
int[] myIntArray = {0, 0, 1, 2, 0, 0, 2, 1, 2, 1, 3, 3, 2, 1, 3, 3}; result = myIntArray[one * 4 + two];
不知道2的位移是否比乘法快。 但这可能是值得的尝试。
这使用了一点bitmagic（你已经通过在一个整数中保存两位信息（低/高＆攻击/块））来做到这一点：
我没有运行它，只在这里input，请重新检查。 这个想法肯定有效。 编辑：它现在每个inputtesting，工作正常。
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; } }
还是应该build议将两位信息分离成单独的variables？ 大部分基于上述位操作的代码通常很难维护。
说实话，每个人都有自己的代码风格。 我不会认为演出会受到太多的影响。 如果您比使用开关盒版本更好地理解这一点，那么继续使用这个。
你可以嵌套ifs，所以如果你的最后一次检查可能会有一点点的性能提升，因为它不会经过很多if语句。 但是，在你的基本Java课程的情况下，它可能不会受益。
else if(one == 3 && two == 3) { result = 3; }
所以，而不是…
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; }
你会做…
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; } }
只要按照你的意愿重新格式化即可。
这不会使代码看起来更好，但是可能会加速一点我相信。
让我们看看我们所知道的
1：你的回答对于P1（玩家一）和P2（玩家二）是对称的。 这对战斗游戏来说是有意义的，但也可以利用它来改善你的逻辑。
2：3节拍0节拍2节拍1节拍3.这些情况下没有涉及的唯一情况是0与1和2与3的组合。换句话说，唯一的胜利表看起来像这样：0 beats 2，1 beats 3次，2次1次，3次，0次。
3：如果0/1互相对抗，那么就会有一个不中的平局，但是如果2/3对每一个都是一样的话，那么两个命中
首先，让我们build立一个单向函数告诉我们，如果我们赢了：
// returns whether we beat our opponent public boolean doesBeat(int attacker, int defender) { int[] beats = {2, 3, 1, 0}; return defender == beats[attacker]; }
然后我们可以使用这个函数来组成最终的结果：
// 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; }
虽然这可以说是比较复杂，可能比许多答案中提供的表查找更慢，但我相信这是一种优越的方法，因为它实际上封装了代码的逻辑并将其描述给正在阅读代码的任何人。 我认为这是一个更好的实施。
（自从我做了任何Java以来，已经有一段时间了，如果语法是closures的，那么希望它仍然可以理解，如果我把它弄错了）
顺便说一下，0-3显然是有意义的; 他们不是任意的价值，所以这将有助于命名他们。
我希望我正确理解逻辑。 感觉如何？
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; }
检查一个高命中或一个低命中不被阻止，对于玩家二来说相同。
编辑：algorithm没有完全理解，“阻击”授予时，我没有意识到（Thx埃利亚斯）：
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; }
我没有Java的经验，所以可能会有一些错别字。 请考虑将代码视为伪代码。
我会去一个简单的开关。 为此，你需要一个数字评估。 但是，对于这种情况，由于0 <= one < 4 <= 9和0 <= two < 4 <= 9 ，所以我们可以通过将one乘以10并将两个int都转换为简单的int。 然后在结果数字中使用开关，如下所示：
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; }
还有一个简短的方法，我只是想指出一个理论的代码。 不过，我不会使用它，因为它有一些额外的复杂性，你通常不想处理。 额外的复杂性来自基地4 ，因为计数是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]; }
真的只是额外的注意事项，以防万一我错过了Java的东西。 在PHP中，我会做：
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]; }
既然你喜欢嵌套， if条件，这是另一种方式。
请注意，它不使用result成员，它不会更改任何状态。
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; }
尝试用开关shell。 ..
看看这里或这里的更多信息
switch (expression) { case constant: statements; break; [ case constant-2: statements; break; ] ... [ default: statements; break; ] ... }
您可以添加多个条件 （不是同时），甚至有一个默认选项 ，其他情况下没有得到满足。
PS：只有满足一个条件
如果两个条件同时出现..我不认为开关可以使用。 但是你可以在这里减less你的代码。
Java开关语句有多种情况
我所遇到的第一件事实质上是由Francisco Presencia给出的同样的答案，但有所优化：
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; } }
你可以进一步优化它的最后一种情况（3）默认情况下：
//case 22: //case 23: //case 32: //case 33: default: return 3;
这种方法的优点是比较容易看出one和two哪个值对应于哪个返回值比一些其他build议的方法。
((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.
希望这可以帮助。
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 triumph over others.
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; } }
例：
System.out.println(Move.compare(Move.ATTACK_HIGH, Move.BLOCK_LOW));
打印：
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 ..
码
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); } }
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 matrix 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; }
- Use constants or enums to make the code more readable
- Try to split the code into more functions
- 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 } }
