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 回答