4

情况如下：

我们有一些通用图形代码用于我们的一个项目。在对代码进行了一些清理之后，似乎有些东西不再起作用（图形输出看起来完全错误）。

我对给出正确输出的最后一个代码版本进行了比较，看起来我们改变了我们的一个函数，如下所示：

static public Rectangle FitRectangleOld(Rectangle rect, Size targetSize)
{
    if (rect.Width <= 0 || rect.Height <= 0)
    {
        rect.Width = targetSize.Width;
        rect.Height = targetSize.Height;
    }
    else if (targetSize.Width * rect.Height > 
        rect.Width * targetSize.Height)
    {
        rect.Width = rect.Width * targetSize.Height / rect.Height;
        rect.Height = targetSize.Height;
    }
    else
    {
        rect.Height = rect.Height * targetSize.Width / rect.Width;
        rect.Width = targetSize.Width;
    }

    return rect;
}

到

static public Rectangle FitRectangle(Rectangle rect, Size targetSize)
{
    if (rect.Width <= 0 || rect.Height <= 0)
    {
        rect.Width = targetSize.Width;
        rect.Height = targetSize.Height;
    }
    else if (targetSize.Width * rect.Height > 
             rect.Width * targetSize.Height)
    {
        rect.Width *= targetSize.Height / rect.Height;
        rect.Height = targetSize.Height;
    }
    else
    {
        rect.Height *= targetSize.Width / rect.Width;
        rect.Width = targetSize.Width;
    }

    return rect;
}

我们所有的单元测试都通过了，除了一些语法快捷方式之外，代码中没有任何变化。但就像我说的，输出是错误的。我们可能只是恢复到旧代码，但我很好奇是否有人知道这里发生了什么。

谢谢。

4

3 回答 3

22

听起来你没有足够的单元测试：]

不幸的是，你的陈述

“除了一些语法快捷方式外，代码中没有任何变化”

错了，我猜这就是你的问题所在。（这当然是你的问题之一！）

是的，

a *= b;

相当于

a = a * b;

但

a *= b / c;

不一样

a = a * b / c;

反而

a *= b / c;    // equivalent to a = a * (b / c)
a = a * b / c; // equivalent to a = (a * b) / c

（参见msdn 上的c# 运算符优先级）

我猜当您的目标高度不是原始矩形高度的精确倍数（或宽度相同）时，您会遇到麻烦。

那么你最终会遇到以下这种情况：

假设 rect.Size = (8, 20), targetSize = (15, 25)

使用您的原始方法，您将得出以下计算：

rect.Width     = rect.Width * targetSize.Height / rect.Height;
//             = 8          * 25                / 20
//             = 200 / 20 (multiplication happens first)
//             = 10
// rect.Width  = 10

使用您的新代码，您将拥有

rect.Width    *= targetSize.Height / rect.Height;
//            *= 25 / 20
//            *= 1 (it's integer division!)
// rect.Width  = rect.Width * 1
//             = 8
// rect.Width  = 8

这是不一样的。（如果目标大小小于原始大小，情况会更糟；在这种情况下，整数除法将导致其中一个维度为 0！）

如果“[你的]单元测试都通过了”那么你肯定需要一些额外的测试，特别是那些处理非整数倍数的测试。

另请注意，您的计算

else if(targetSize.Width * rect.Height > 
        rect.Width * targetSize.Height)

不可靠；对于非常大的矩形，它有可能溢出并给你不正确的结果。作为乘法的一部分，您最好转换为更大的类型（即长）。（同样，应该有一些单元测试来达到这个效果）

希望有帮助！

于 2009-02-13T21:07:04.493 回答
6

如果 Rectangle.Width 和 Rectangle.Height 是整数，则以下两行不同：

rect.Width = rect.Width * targetSize.Height / rect.Height;
rect.Width *= targetSize.Height / rect.Height;

第一行按顺序执行乘法、除法、强制转换为整数，然后是赋值。第二个执行除法，强制转换为整数，乘法，然后赋值。问题是，在您的非工作代码中，您的 divide 在 multiply 之前被转换为整数。

保留原始代码或强制除法为浮点。

编写更好的单元测试来检查这个问题。（尝试没有偶数倍数的宽度/高度组合（例如素数）。）

于 2009-02-13T21:08:14.490 回答
0

添加到 Daniel L 的答案。

这种“优化”的意义何在？有更好的方法来清理此代码，并使其更具可读性。

于 2009-02-13T21:11:17.010 回答