这部分看起来足够干净吗?关于如何使其更清洁的任何建议?

if (isCheck)
{
    if (isStuck)
    {
        return GameState.Mate;
    }
    else
    {
        return GameState.Check;
    }
}
else
{
    if (isStuck)
    {
        return GameState.Stalemate;
    }
    else
    {
        return GameState.Ok;
    }
}


评论

我认为这是省略(某些)花括号可以改善您的代码的情况之一。

我认为没有任何答案比原始代码更具可读性。而且看起来它已经是一个独立的方法,因此在其他地方,您只是调用GetGameState(isCheck,isStuck)。

我认为您不需要此枚举。枚举是一个中间值,而不是目标。仅使用IsStuck和IsCheck,您的代码就可以:if(isStuck){displayReason(); GameOver();} ...否则...如果(IsCheck){SayCheck(); }。如果生成枚举,则仍然需要对它运行一堆条件。

@James占满整个屏幕的一半,可以整齐地写成三行(嵌套三进制)。我不明白为什么还要讨论这些解决方案的相对优势。

#1 楼

我将在此处使用决策表:

GameState[,] decisionTable = new GameState[2,2];
decisionTable[0,0] = GameState.Ok;
decisionTable[0,1] = GameState.Stalemate;
decisionTable[1,0] = GameState.Check;
decisionTable[1,1] = GameState.Mate;
return decisionTable[Convert.ToInt32(isCheck), Convert.ToInt32(isStuck)];


摘自Code Complete第二版,第19章:常规控制问题,第431页:


使用决策表来替换复杂的条件

有时您具有包含多个变量的复杂测试。使用决策表执行测试而不是使用if或case可能会有所帮助。决策表查找起初更容易编码,只需要几行代码,而没有棘手的控制结构。复杂性的这种最小化使出错的机会最小化。如果数据发生更改,则可以更改决策表而无需更改代码。您只需要更新数据结构的内容。


评论


\ $ \ begingroup \ $
只要记住将其隔离在一个函数中即可。
\ $ \ endgroup \ $
–luiscubal
13年2月17日在18:43

\ $ \ begingroup \ $
我认为,只有两个这样的情况,实际上很难遵循决策树。 OP代码的更简洁的表示将更加简洁和易于理解。
\ $ \ endgroup \ $
–杰克·艾德利(Jack Aidley)
13年2月18日在10:40

\ $ \ begingroup \ $
palacsint,我可以自由地将您的伪代码转换为真实的C#编译器。如果您有任何异议,请随时回滚我的编辑。
\ $ \ endgroup \ $
–亚当
13年2月21日在20:25



\ $ \ begingroup \ $
@codesparkle:我很高兴有人做到了:)谢谢!
\ $ \ endgroup \ $
–palacsint
13年2月21日在21:39

#2 楼

我认为?运算符将使您的代码更紧凑和可读性:

return isCheck
    ? isStuck ? GameState.Mate : GameState.Check
    : isStuck ? GameState.Stalemate : GameState.Ok


评论


\ $ \ begingroup \ $
这还不错。但是我发现三元运算符嵌套的速度比其他运算符快得多,并且很难避免这样做。
\ $ \ endgroup \ $
–丹在火光中摆弄
13年2月17日在17:18

\ $ \ begingroup \ $
这就是我总是写的方式。
\ $ \ endgroup \ $
–康拉德·鲁道夫(Konrad Rudolph)
13年2月18日在8:18

\ $ \ begingroup \ $
为了清楚起见,我将代码放在括号中,但是我认为这完全可以接受。
\ $ \ endgroup \ $
–杰克·艾德利(Jack Aidley)
13年2月18日在10:41

\ $ \ begingroup \ $
@DanNeely同意您的意见,但是在这种情况下1级嵌套似乎是合理的。对于我来说,决策表在这里是一个过大的杀伤力,特别是考虑到您需要先映射bool-> int或使用Dictionary直接映射布尔值
\ $ \ endgroup \ $
– almaz
13年2月18日在10:51

#3 楼

作为决策表@palacsint解决方案的替代变体,一系列bool ^ n的情况通常使用标志来表示。

// in a global constants file somewhere
const Stuck = 1 << 0; // 1
const Check = 1 << 1; // 2
const ..... = 1 << n; // 2^n

int decisionTable[] = {
    GameState.Ok,         // 0b00
    GameState.Stalemate,  // 0b01
    GameState.Check,      // 0b10
    GameState.Mate,       // 0b11
};


// in code
int flags = 0;
if (isStuck(board)) flags |= Stuck;
if (isCheck(board)) flags |= Check;

return decisionTable[flags];


作为带有一系列参数的函数(例如getState(isCheck, isStuck)),您必须记住参数的精确顺序,当您拥有大量标志时,这将变得很笨拙。例如,如果您不小心交换了以下索引:decisionTable[(int)isStuck][(int)isCheck];,您可能会发现自己陷入了漫长的调试会话,而从未意识到交换。按位或是可交换的,因此没有此问题。

评论


\ $ \ begingroup \ $
诚实地看一下OP的代码,我比该代码更好理解。位移在许多不同的上下文中都有其位置,但我认为这一点没有用。
\ $ \ endgroup \ $
– Robert Snyder
13年2月17日在17:55

\ $ \ begingroup \ $
@RobertSnyder:您只需要编写一次这些全局常量,但是您使用标志和标志表的频率更高。位移的一种替代方法是使用power(2,n),但是IMO位移比pow()更清楚地转换意图,因为标志不是真正的整数/数字,而是一系列的布尔值(一系列的位)。
\ $ \ endgroup \ $
– Lie Ryan
13年2月17日在23:54



\ $ \ begingroup \ $
C#的最新版本支持命名参数,因此您可以编写类似getGameState(isStuck:this.isStuck,isCheck:this.isCheck)的内容。然后,确切的参数顺序无关紧要。
\ $ \ endgroup \ $
–用户
13年2月18日在10:18



#4 楼

首先,我认为您的解决方案不错。正如许多其他各种各样的答案所暗示的那样,这只是感觉应该具有简单,优雅,单线风格解决方案的问题之一。但是,事实并非如此。有较短的解决方案,但它们都牺牲了可读性。

就像我在上面说的那样,我认为您的方法很好,而且如果我在工作中进行审查的代码中遇到它,我也不会打扰。对它发表评论。但我确实认为可以将其清除一点。当接近嵌套的if时,我认为最好从外部if中最宽泛的问题开始,然后再问以下更狭窄的问题。另外,我认为通常将最常见的情况放在顶部通常会更好,因为当某人正在阅读代码(也许试图调试问题)时,他们最有可能对此分支感兴趣……在我看来,这似乎更合乎逻辑。最后,不要惧怕添加一个命名良好的变量,只是为了使代码更具可读性。

考虑到所有这些,这是我的解决方案:

        var gameShouldContinue = ! isStuck;
        if (gameShouldContinue) {
            return isCheck ? GameState.Check : GameState.Ok;
        } else {
            return isCheck ? GameState.Mate : GameState.Stalemate;
        }


评论


\ $ \ begingroup \ $
“解决方案虽然较短,但都牺牲了可读性。” –错。嵌套三元解决方案没有。它在任何语言中都是惯用语言,可以直接传达其功能。
\ $ \ endgroup \ $
–康拉德·鲁道夫(Konrad Rudolph)
13年2月18日在17:05

\ $ \ begingroup \ $
嗯...在这种情况下,嵌套的条件运算符如果格式正确,将不会很难读,但是作为一般经验法则,我认为这是不好的做法。不是因为无法理解它,而是因为它花费了更多的时间来理解并且几乎没有收益。青年汽车
\ $ \ endgroup \ $
–安迪
13年2月18日在22:12

\ $ \ begingroup \ $
不过,我们在这里讨论的是一层嵌套的具体情况。这没有任何问题-通常,如果格式正确,链接三元运算符就很难读(PHP除外,因为该语言会吸收错误并获得错误的优先级)-实际上,它读起来就像是switch语句。这是C ++中的示例。同样,这是大多数语言中的惯用代码。
\ $ \ endgroup \ $
–康拉德·鲁道夫(Konrad Rudolph)
13年2月19日在11:54



\ $ \ begingroup \ $
@KonradRudolph我要说的一件事是,它比嵌套的条件运算符更易于阅读(至少使用C#的丑陋?:语法)。
\ $ \ endgroup \ $
– CodesInChaos
13年2月19日在15:25

\ $ \ begingroup \ $
@KonradRudolph您的示例给我留下了深刻的印象,并同意它的可读性。但是,我认为这在这里不适用。在这种情况下,GameState取决于两个变量,在您的示例中,只有一个。这里的嵌套条件将需要外部条件的真分支和假分支都包含嵌套条件运算符。
\ $ \ endgroup \ $
–安迪
13年2月19日在22:49

#5 楼

这取决于您认为更具可读性的内容,但是您可以使用类似于二进制的方式在真值表中表示变量:

isCheck isStuck   Value
=======================
T       T         Mate
T       F         Check
F       T         Stalemate
F       F         Ok


这将转换为某些内容更紧凑:

if (isCheck  && isStuck)    return GameState.Mate;
if (isCheck  && !isStuck)   return GameState.Check;
if (!isCheck && isStuck)    return GameState.Stalemate;
if (!isCheck && !isStuck)   return GameState.OK;


评论


\ $ \ begingroup \ $
实际上,这比原始代码要复杂得多且容易出错。原因如下:四个if而不是三个;四个2字母的布尔表达式,而不是三个1字母的布尔表达式;相互排斥在结构上不明显。
\ $ \ endgroup \ $
– Rotsor
13年2月17日在17:32

\ $ \ begingroup \ $
基于这些指标,我可以同意其复杂性。但是,以设计模式为例:可以说它们以类数的形式引入复杂性吗?当然可以。这就是为什么我指出设计决策来自何处:真值表。就像您对用于Java流的许多类感到困惑一样,当您发现它们遵循装饰器模式时,所有一切突然变得更加清晰。我提出了这个答案,因为我迫切需要那种最重视清晰度和兼容性的功能。
\ $ \ endgroup \ $
–哈维尔·维拉
13年2月18日在0:12



\ $ \ begingroup \ $
由于return实际上会返回,因此您可以跳过所有其他的else关键字,以使冗长一些:
\ $ \ endgroup \ $
–Svish
13年2月18日在10:12

\ $ \ begingroup \ $
@JavierVilla:您可以从测试3中删除isCheck,然后将测试4转换为简单的返回值。这使其不比原始版本复杂。
\ $ \ endgroup \ $
– jmoreno
13年2月18日在18:44

\ $ \ begingroup \ $
一个问题是,您需要在四个if下方添加一些内容,因为编译器无法理解它们是否处理了所有可能的情况。
\ $ \ endgroup \ $
– CodesInChaos
13年2月19日在15:27

#6 楼

每当您有多个代表状态的独立布尔变量时,请考虑通过应用已应用FlagsAttribute的枚举来使用位域。这样,您无需维护多个变量即可代表一个状态。

[Flags]
public enum BoardState
{
    None = 0x0,
    IsCheck = 0x1,
    IsStuck = 0x2,
}


然后您可以将这些棋盘状态映射到适当的游戏状态。如果打算使用字典,只需确保映射出所有有效的组合即可。

private Dictionary<BoardState, GameState> transition = new Dictionary<BoardState, GameState>
{
    { BoardState.None,                         GameState.Ok },
    { BoardState.IsCheck,                      GameState.Check },
    { BoardState.IsStuck,                      GameState.Stalemate },
    { BoardState.IsCheck | BoardState.IsStuck, GameState.Mate },
};
public GameState Next(BoardState boardState)
{
    GameState nextState;
    if (!transition.TryGetValue(boardState, out nextState))
        throw new ArgumentOutOfRangeException("boardState");
    return nextState;
}


否则,如果映射太不规则或要映射,请使用switch语句将多个组合组合为一个值(在这种情况下是不必要的)。如果您具有代表多个值的值,请务必小心使用此方法。

public GameState Next(BoardState boardState)
{
    switch (boardState)
    {
    case BoardState.None:
        return GameState.Ok;
    case BoardState.IsCheck:
        return GameState.Check;
    case BoardState.IsStuck:
        return GameState.Stalemate;
    case BoardState.IsCheck | BoardState.IsStuck:
        return GameState.Mate;
    default:
        throw new ArgumentOutOfRangeException("boardState");
    }
}


您可以清理并保持其良好状态,并通过布尔属性对其进行清理。

BoardState boardState = ...;
// to set a flag
boardState |= BoardState.IsCheck;
// to clear a flag
boardState &= ~BoardState.IsStuck;
// to test a flag
boardState.HasFlag(BoardState.IsCheck);


#7 楼

只需添加@palacsint的答案,如果您使用的语言允许数组文字,则可以使其看起来像一个表:

var decisionTable = new GameState[][] {
  new GameState[] { GameState.Checkmate, GameState.Check } // isCheck
  new GameState[] { GameState.Stalemate, GameState.Ok    }
                      // isStuck
}
return decisionTable[(int)isCheck][(int)isStuck];


评论


\ $ \ begingroup \ $
如果您认为他的论点只是基本的二进制计数练习,我想您甚至可以进一步简化。话虽这么说,如果您可以将isCheck和isStuck更改为简单的标志枚举,则可以对这些数字进行OR运算并返回适当的GameState。
\ $ \ endgroup \ $
– Robert Snyder
13年2月17日在18:00

\ $ \ begingroup \ $
不要忘记原始的@palacsint答案是伪代码,您不能将bool转换为int
\ $ \ endgroup \ $
– almaz
13年2月18日在10:55

#8 楼

内联if语句较短,但我不了解清洁器

return isCheck
       ? (isStuck ? GameState.Mate : GameState.Check)
       : (isStuck ? GameState.Stalemate : GameState.Ok);


并按位扩展

int result = (isCheck ? 1 : 0) + (isStuck ? 2 : 0);
return (GameState) result;

enum GameState
{
    Ok = 0,
    Check = 1,
    Stalemate = 2,
    Mate = 3,
}


您可以将枚举“用作”决策树!

我安排了它们

#9 楼

对于这种简单的情况,我认为决策树不是必需的,并且过于复杂-尽管如果决策树比这更复杂,它们具有明显的优势,但我个人愿意这样做: br />与纯if解决方案相比,我发现它比纯三元运算符解决方案更清晰,更紧凑,更易读。

评论


\ $ \ begingroup \ $
您的其他人在这里多余。
\ $ \ endgroup \ $
– Vadim Ovchinnikov
18 Mar 24 '18在11:43

\ $ \ begingroup \ $
@VadimOvchinnikov你是正确的。但是,我认为它使代码的流程和意图更易于阅读,因此我将其保留下来。
\ $ \ endgroup \ $
–杰克·艾德利(Jack Aidley)
18年3月24日在12:04

#10 楼

我认为无需多说,

             stuck
          |--------- mate (11)
   check  |
|---------|
|         |
|         |--------- check (10)
|          
|            stuck
|         |--------- stalemate (01)
|         |
|---------|
          |
          |--------- ok (00)

enum GameState
{
  Ok = 0, // 0x00
  Stalemate = 1, // 0x01
  Check = 2, //0x10
  Mate = 3 //0x11
}

return (GameState) (0x10 * isCheck + isStuck);


#11 楼

如果您喜欢决策表的想法(例如在palacsint的答案中),则可以使用ValueTupleDictionary组合使用更优雅的方法。


#12 楼

我认为决策表已经足够了。如果您想深奥,也可以将枚举值定义为位值,并且映射将是自动的(这是特定于给定示例的,因此通常不是一个好主意):

enum GameState
{
  Ok = 0,
  Stalemate = 1,
  Check = 2
  Mate = 3
}


代码变为:

return (GameState)(((int)isCheck << 1)|(int)isStuck);


评论


\ $ \ begingroup \ $
他为什么要“神秘”?
\ $ \ endgroup \ $
–本杰明·格林巴姆
13年2月18日在18:21

\ $ \ begingroup \ $
他可能不会。但是,知道所有其他选择可能会在将来寻找类似问题的解决方案时扩大我们的视野。对于不同的约束集,这可能是一个完美的解决方案。
\ $ \ endgroup \ $
– Sedat Kapanoglu
13年2月18日在21:15

#13 楼

您可以简单地使GameState成为Flags枚举,它只是palacsint建议的决策表的更紧凑版本。

带有按位或运算符的标志:

[Flags]
public enum GameState
{
    None = 0x0,
    IsCheck = 0x1,
    IsStuck = 0x2,

    Ok = None,
    Check = IsCheck,
    Stalemate = IsStuck,
    Checkmate = IsCheck | IsStuck,
}


如果要检查其上的两个位标志,则可以按如下方式使用按位运算符:

GameState current = GameState.Ok;
current |= GameState.IsStuck;
current |= GameState.IsCheck;


,或者,在最新版本的.NET中,可以使用Enum.HasFlag方法:

是您对标志的所有读/写操作完全是通过存储GameState的变量完成的,而不是使用独立的布尔值和周围的多维数组来查找事物。

#14 楼

这么多疯狂的答案!我再加一个! :D

良好代码的最重要属性是没有代码重复。在原始解决方案中,这是if (isStuck)条件,该条件已重复。我们可以通过巧妙地(在这种情况下不多)抽象来避免代码重复!

赢了!

#15 楼

就我个人而言,我只是将树折叠起来。

比尝试转换为枚举更具可读性,并且比决策网格更直接。话虽这么说,但它并不能同时扩展。

#16 楼

并不是真的建议这样做,但是您可以使用以下事实:始终使用两个变量来生成四个状态的字符串表示形式,然后使字典或switch语句返回正确的值。

string checkStuck = isCheck.ToString() + isStuck.ToString();
switch(checkStuck) {
   case "TrueFalse":
        return GameState.Mate;
   case "TrueTrue":
       return GameState.Check;
   case "FalseFalse":
        return GameState.Ok;
   case "FalseTrue":
        return GameState.Stalemate;
}


您可以对整数进行相同的操作,但是此时您也可以使用Andy的答案。

评论


\ $ \ begingroup \ $
对于所有未来的开发人员,请不要使用这样的字符串。
\ $ \ endgroup \ $
–杰夫·梅卡多(Jeff Mercado)
13年2月18日在8:46

\ $ \ begingroup \ $
@JeffMercado:您的反对意见是什么?我不想捍卫它,这不是我通常会实现的目标。它真的没有什么可推荐的,只是对于那些没有其他选择经验的人来说可能具有可读性。但这似乎也不是字符串的滥用。
\ $ \ endgroup \ $
– jmoreno
13年2月19日在5:52

\ $ \ begingroup \ $
反对意见显然是该代码实际上是无法辩护的。它效率低下且不雅致。
\ $ \ endgroup \ $
–老师吉尔特
13年2月20日在18:05

\ $ \ begingroup \ $
明确表明需要在C#中进行模式匹配。
\ $ \ endgroup \ $
– Rotsor
13年2月20日在19:08