【问题标题】:Is this kind of a bool method a bad practice?这种 bool 方法是不好的做法吗?
【发布时间】:2016-07-15 10:34:00
【问题描述】:

有时我发现自己编写了一个如下所示的 bool 方法:

public bool isRunning()
    {
        if (!(move == Moving.None) && staminaRegan == true)
        {
            if (keyState.IsKeyDown(Keys.Space))
            {
                EntityAnimation.interval = 10;
                return true;
            }
            else
            {
                EntityAnimation.interval = 65;
                return false;
            }
        }
        else 
        {
            EntityAnimation.interval = 65;
            return false;
        }
    }

(顺便说一下,这是 XNA)如您所见,我有一个 bool isRunning 语句,其中我做了一个 if 语句,我在其中检查是否(玩家正在移动)&&(恢复耐力,一旦耐力达到,它就设置为 false值小于 6.0f) 然后我简单地检查是否按下了 Space,如果是,那么我的 Animation 更快(间隔越小,spritesheet 变化越快),然后它发送 true 值,这意味着 Player 正在运行,否则我不会导致 Space 是没有按下。

然后我必须在第一个 if 语句之外重复这个“else”代码,以便如果玩家没有移动或他的耐力 Regan 为假,它会发送玩家没有运行;

所以我只是想知道这种 bool 方法是否被认为是一种不好的做法(在嵌套 if 中返回 true 和 false 值,然后在嵌套 if 之外返回 false 并重复相同的代码)?

【问题讨论】:

    标签: c# performance xna


    【解决方案1】:

    该方法有一个副作用,这就是为什么它是一个不好的做法

     public bool isRunning()
    

    在查看方法的签名时,我们只希望 true/false 回答,仅此而已。然而,方法改变实例的状态

      ...
      if (!(move == Moving.None) && staminaRegan == true)
        {
            if (keyState.IsKeyDown(Keys.Space))
            {
                EntityAnimation.interval = 10; // <- Aaa! The interval is changed 
                return true;
            }
      ...
    

    我建议将初始方法拆分为属性和方法

     // No side effect: just answer is running or not
     public bool IsRunning {
       get {
         return (move != Moving.None) && staminaRegan && KeyState.IsKeyDown(Keys.Space);
       }
     }
    
     // Put the right interval based on instance internal state 
     // (if it's running etc.)
     public void AdjustInterval() {
       if (IsRunning) // and may be other conditions
         EntityAnimation.interval = 10; //TODO: move magic number into constant 
       else 
         EntityAnimation.interval = 65; //TODO: move magic number into constant
     } 
    

    【讨论】:

    • 为什么我不应该在 bool 方法中更改一些(其他类-公共静态变量)值,我只是在将 true 或 false 返回到 bool 之前更改一些东西:/
    • 因为每个方法都应该只做一件事。您应该创建一个方法 IsRunning 和一个方法 AdjustInterval。如果您愿意,可以使用另一种方法 CheckRunningAndAdjustInterval
    • (所有这些和 )没有惊喜!
    【解决方案2】:

    在方法中包含一个 return 语句是一种很好的做法。有人对此争论不休,但这是一种观点。

    通过删除不必要的代码来明确 if 语句也是一个好习惯:

    public bool isRunning()
    {
        bool result = false;
        if (move != Moving.None && staminaRegan)
        {
            if (keyState.IsKeyDown(Keys.Space))
            {
                EntityAnimation.interval = 10;
                result = true;
            }
            else
            {
                EntityAnimation.interval = 65;
            }
        }
        else
        {
            EntityAnimation.interval = 65;
        }
    
        return result;
    }
    

    【讨论】:

    • 我同意有一个退货声明,它位于底部,看起来更干净。并且看到您最初已经将结果设置为 false,因此不需要在方法中进一步重置它 - 增加了可读性。
    • 在方法中包含一个 return 语句是一种很好的做法。 No. 将代码强制拟合到与正在执行的实际逻辑不匹配的特定模式中是什么不好。另见goodreads.com/quotes/…
    • 是的,只有一份退货声明是一种很好的做法,甚至在我工作的大多数公司中都需要它。代码更具可读性,但更重要的是更好地维护。我已经看到人们搜索了几个小时,为什么在方法底部的 retun 语句之前新添加的 logmethod 没有被执行。
    【解决方案3】:

    你可以重写代码如下;然后代码不再重复:

    public bool isRunning()
    {
        if (move != Moving.None && staminaRegan && keyState.IsKeyDown(Keys.Space))
        {
            EntityAnimation.interval = 10;
            return true;
        }
        else
        {
            EntityAnimation.interval = 65;
            return false;
        }
    }
    

    或者如果你不想要多余的else

    public bool isRunning()
    {
        if (move != Moving.None && staminaRegan && keyState.IsKeyDown(Keys.Space))
        {
            EntityAnimation.interval = 10;
            return true;
        }
    
        EntityAnimation.interval = 65;
        return false;
    }
    

    我会考虑在自我文档中引入一个命名布尔值,我会将 staminaRegan 重命名为 staminaIsRegenerating

    public bool isRunning()
    {
        bool isMovingQuickly = (move != Moving.None) && staminaIsRegenerating && keyState.IsKeyDown(Keys.Space);
    
        if (isMovingQuickly)
            EntityAnimation.interval = 10;
        else
            EntityAnimation.interval = 65;
    
        return isMovingQuickly;
    }
    

    不过,最重要的是,您应该重命名该方法以更准确地描述它的作用:

    public bool CheckIfRunningAndSetAnimationInterval()
    

    【讨论】:

      【解决方案4】:

      我认为我们是为人(其他开发人员)编写代码,当然是机器执行代码,但开发人员 80% 的工作是读取代码。
      基于此,我认为阅读流程必须与执行代码流程完全相同 - 这就是为什么我认为将 return 语句相乘并不是一件坏事,甚至比方法底部只有一个 return 语句更好。

      【讨论】:

      • 编译器应该能够识别多个返回语句并且它应该给出一个错误。这太难读了,是编程历史上最大的错误提供者
      • @GuidoG:您是否有任何参考资料来支持您的论点,即多个返回语句是“编程历史上最大的错误供应商”。我经常看到复杂的、令人费解的代码在尝试实现单个 return 语句时引入了错误——如果有多个 return 语句,代码会大大简化且更易于维护。
      • 如果方法一开始只做一件事,那么我们就不会有多重返回语句的问题。
      • @PaulF Ever 刚读完所有关于结构化编程的书。在一种方法中拥有多个出口被认为是不好的做法,在我看来它仍然是。是的,结构化编程很古老,但 oop 受到了它的启发
      • @GuidoG:关于 SESE 是好还是坏的做法以及它是否仍然适用于许多更现代的语言,有很多争论——我只是想阅读任何关于它是“编程史上最大的错误供应商”。
      【解决方案5】:

      我喜欢这种风格,我也使用它。首先,您可以更轻松地阅读代码,其次它具有调试优势,因为您可以为个别 else 情况设置断点。否则你需要使用断点条件。

      【讨论】:

        猜你喜欢
        • 1970-01-01
        • 1970-01-01
        • 1970-01-01
        • 2011-07-07
        • 1970-01-01
        • 1970-01-01
        • 1970-01-01
        • 1970-01-01
        • 1970-01-01
        相关资源
        最近更新 更多