4

我有一些几周前写的代码（代码的目的不如它的结构重要）：

if (_image.Empty)
{
   //Use the true image size if they haven't specified a custom size
   if (_glyphSize.Width > 0)
      imageSize.Width = _glyphSize.Width //override
   else
      imageSize.Width = _image.GetWidth;

   if (_glyphSize.Height > 0) then
      imageSize.Height = _glyphSize.Height
   else
      imageSize.Height = _image.GetHeight
}
else
{
   //No image, but they can still override it with a custom size
   if (_glyphSize.Width > 0) then
      imageSize.Width = _glyphSize.Width
   else
      imageSize.Width = 0;

   if (_glyphSize.Height > 0)
      imageSize.Height = _glyphSize.Height
   else
      imageSize.Height := 0;
}

今晚我正在检查它，当我清理它时，我意识到清理后的版本必须更简洁：

//Figure out the final image width
if (_glyphSize.Width > 0)
   imageSize.Width = _glyphSize.Width
else if (not _glyph.Empty)
   imageSize.Width = _glyph.GetWidth
else
   imageSize.Width = 0;

//Figure out the final image height
if (_glyphSize.Height > 0)
   imageSize.Height = _glyphSize.Height
else if (not _glyph.Empty)
   imageSize.Height = _glyph.GetHeight
else
   imageSize.Height = 0;

注意：我已将代码精简为裸露的逻辑流程，并混淆了源语言。

最后，我采用了嵌套if的 's，并将它们倒置。这样做允许这种缩短。我的问题是：我以后如何才能认识到这一点？

有什么迹象表明我刚刚编写了一些可以重构为更短代码的代码？

几周前的另一个例子类似于权限检查：用户可以执行一个操作：

  • 如果他们有许可，他们可以这样做
  • 如果他们没有权限，但覆盖有效

我最初编码为：

if ((HasPermission || (!HasPermission and OverrideEnabled))
{
   ...do stuff
}

该if条款的逻辑条件似乎有点冗长。我试图回到我的布尔代数课程来弄清楚如何简化它。最后我可以做到，所以我最终绘制了一个真值表：

Permission  Override   Result
    0           0        0
    0           1        1
    1           0        1
    1           1        1

当我看它时，它是一个OR操作。所以我的if声明变成了：

if (HasPermission  or OverrideEnabled)
{
   ...
}

这是显而易见的和简单的。所以现在我想知道我怎么看不到这一点。

这让我回到了我的 SO 问题：为了识别某些代码块需要一些 TLC，我可以/应该寻找什么迹象？

4

4 回答 4

2

这里有一些来自 Code Complete 的指导方针，我想不到。对于这类事情，这是一本好书。

  1. 在块中嵌套 if-else 和重复语句
  2. 长 for 循环
  3. 重复的行/语句或经常使用的操作可以放在一个函数中
  4. 如果由于某些原因您一遍又一遍地复制和粘贴一行代码

我发现离散数学对我现在编写 if 语句的方式有影响。通常，我看到我在 2 个块中编写了两个相同的 IF 语句，然后我会做一些心理“分解”。

于 2010-01-23T03:13:55.070 回答
1

特别与布尔求值相关，值得注意的是大多数（？）现代语言都实现了惰性求值。

也就是说，如果“a”为真，那么if(a)和if(a or b)在逻辑上和功能上是等价的；解释器在看到or一个真正的变量时停止评估。a当和是变量时这不是很重要b，但如果它们是可调用的[例如if(a() or b())]，b()则不会在a为真时被评估。

通过好好学习，您可以节省大量击键（和处理器时间）：

if(!userExists()):
    if(!createUser()):
        errorHandling()
    else:
        doStuff()
else: doStuff()

变成

if(userExists() or createUser()): doStuff()
else: errorHandling()
于 2010-01-23T03:28:08.680 回答
1

做得好。现在，当我看到这个时：

//Figure out the final image width
if (_glyphSize.Width > 0)
...

//Figure out the final image height
if (_glyphSize.Height > 0)
...

我认为还有更多的重构要做。将代码提取到方法中不仅仅是消除冗余代码的好方法。这也是使代码自我记录的好方法：

我倾向于将代码减少到：

set_final_image_size

使用 set_final_image_size 及其仆从定义如下：

def set_final_image_size:
  imageSize.Width = final_image_width;
  imageSize.Height = final_image_height;

def final_image_width:
  if (_glyphSize.Width > 0)
     return _glyphSize.Width;
  else if (not _glyph.Empty)
     return _glyph.GetWidth;
  else
     return 0;

def final_image_height:
  if (_glyphSize.Height > 0)
     return _glyphSize.Height;
  else if (not _glyph.Empty)
     return _glyph.GetHeight;
  else
     return 0;
于 2010-01-23T03:55:06.310 回答
0

现在您已经分离了宽度和高度逻辑，并注意到它是相同的 - 如果您要添加，比如说，getDimension(Direction direction)您setDimension(Direction direction, int length)的类怎么办？现在你有

if (_glyphSize.getDimension(direction) > 0)
   imageSize.setDimension(direction, _glyphSize.getDimension(direction))
else if (not _glyph.Empty)
   imageSize.setDimension(direction, _glyph.getDimension(direction))
else
   imageSize.setDimension(direction, 0);

提取本地给我们带来：

length = _glyphSize.getDimension(direction);
if (length > 0)
   imageSize.setDimension(direction, length)
else if (not _glyph.Empty)
   imageSize.setDimension(direction, _glyph.getDimension(direction))
else
   imageSize.setDimension(direction, 0);

更进一步：

length = _glyphSize.getDimension(direction);
if (length == 0 && !_glyph.Empty)
    length = _glyph.getDimension(direction);
imageSize.setDimension(direction, length);

至少在我看来，它开始看起来很漂亮。

于 2010-01-23T17:04:52.067 回答