绘制和绘制菜单组件

Question

我已经为我正在做的一个项目生成了一个工作菜单组件，但是我想减少使用的代码数量并改进方法--特别是在油漆实现中--更进一步，我觉得我所使用的函数和将变量传递给它的方式有点“麻烦”-- IOW，我给它做了个补充。

我可以用什么方法来增加绘图例程中使用的封装量，这样我就不会在画图例程中定义变量了？尤其是y2变量是令人恼火的。

这些是我的绘图功能和绘图例程：

// Rectangle drawing function
Function DrawRect(cv : TCanvas; x1, y1, x2, y2, chR, clR, i : integer; txts : AnsiString; Lrect : TRect) : TRect;
begin
  with cv do begin
    // Test against Rect variables to set colour
    if (chR = i) and (clR = -1) then
      Brush.color := stateColor[bttn_on]
    else if (chR = i) and (clR = i) then
      Brush.color := stateColor[bttn_dwn]
    else
      Brush.color := stateColor[bttn_off];

    // Define Link Rectangle
    Lrect := Rect(x1, y1, x2, y2*(i+1));

    // Draw Rectangle
    Rectangle(Lrect);

    // Add text
    TextRect(Lrect, x1+5,  y1+5, txts);

    // Return co-ordinates of drawn rectangle as function result
    result := Lrect;
  end;
end;

// Canvas painting
procedure TOC_MenuPanel.Paint;
var
  y1, x2, y2, count : Integer;
begin
  inherited Paint;
  // Set length of array
  SetLength(MenuRects, fLinesText.Count);

  // Define Y1,Y2
  x2 := Width - Width div 20;
  y1 := Width div 20;

  with Canvas do begin
  // Draw outerRectangle outside of For loop
  Brush.color := clMenu;
  Rectangle(0, 0, Width, Height);

  // Loop for drawing menu items
  if fLinesText.Count = 0 then exit
  else
    for count := 0 to fLinesText.Count - 1 do begin
      // Define Y2
      y2 := TextHeight(fLinesText.strings[count])*2;

      // Cast rectangle draw function results to array for Mouse actions & draw rectangle
      MenuRects[count] := DrawRect(Canvas, 10, y1, x2, y2, chosenRect, clickedRect, count, fLinesText.strings[count], MenuRects[count]);

      // inc y1 for positioning the next box
      y1 := y1+y2;
    end;
  end;
end;

Answer 1

首先，我将继续讨论代码的外观，因为它显示了最明显的问题。

--到处都是多余的评论.

// Rectangle drawing function
Function DrawRect

名称DrawRect足够清楚地表明，您正在那里画一个矩形。不需要发表评论。

同样适用于：

// Define Y2
y2 := TextHeight(fLinesText.strings[count])*2;

如属下列情况：

// Define Y1,Y2
x2 := Width - Width div 20;
y1 := Width div 20;

这不仅是多余的，而且也是错误的，因为您没有在任何地方设置Y2的值。

代码应该是自我记录的，注释不应该解释你在做什么，最多应该解释你为什么要做你正在做的事情。否则，他们只是把文件弄得乱七八糟，需要额外的工作来维护和看上去很难看。

您应该致力于改进变量的名称.

// Define Link Rectangle
Lrect := Rect(x1, y1, x2, y2*(i+1));

通过调用Lrect LinkRectangle或至少调用LinkRect，您还可以放弃分配上面的注释。LinkRect := Rect(...);清楚地表明，您正在设置一个链接矩形。

chR和clR是一些非常混乱的名称，除非您在实际调用中查看传递给函数的参数，否则很难理解它们应该做什么。以chosenRect和clickedRect为例。

bttn_dwn可以变成button_down (如果这是您决定应用于似乎没有符号的整数常量的样式)。txts可转化为text。

count更常用于表示特定集合中的项数。对于循环计数器，请考虑使用更常见和更合适的名称。在您的特殊情况下，将count重命名为i不应该有问题。

等等..。

在用大小写表示名字时有一些不一致之处.

尽管Lrect和所有其他参数一样是一个参数，但它的大小写却不同。

您选择使用Function，同时也使用procedure。考虑将Function更改为function，或者将procedure更改为Procedure，这取决于您想要如何保持一致性。就我个人而言，我不喜欢从大写字符开始。主要是因为我也没有用一个开始剩下的关键字，而且我看到你也这么做了。

您应该尝试在代码中保持这些内容的一致性。选择一种方式来命名你的东西，并保持它在你的整个项目。

您可以改进代码的行为方式.

if fLinesText.Count = 0 then exit
  else

很有可能成为if fLinesText.Count > 0 then。

但是，这也是多余的，因为如果for count := 0 to fLinesText.Count - 1 do begin小于1，Count将什么也不做。

考虑提取

if (chosenRect = i) and (clickedRect = -1) then
  Brush.color := stateColor[bttn_on]
else if (chosenRect = i) and (clickedRect = i) then
  Brush.color := stateColor[bttn_dwn]
else
  Brush.color := stateColor[bttn_off];

转换为返回stateColor元素所具有的类型的值的函数。然后从TOC_MenuPanel.Paint调用它，并将结果作为参数传递给DrawRect。这将减轻DrawRect上的许多参数造成的负担(您将能够摆脱chosenRect和clickedRect)。它还将从DrawRect中移除知道已选择和单击的矩形的责任。

在下列情况下：

Lrect := Rect(x1, y1, x2, y2 * (i + 1));

您似乎在根据菜单中矩形的索引更改矩形的大小。DrawRect甚至没有理由知道有一个菜单，并且正在绘制的矩形中有一个索引。y2的原始值不在其他地方使用，因此考虑将y2 * (i + 1)作为参数传递(从MenuPanel.Paint中)，并将i作为参数处理。如果您在一个单独的函数中提取颜色选择位，并从Paint调用它，那么这样做也是很简单的。

考虑将x1 x2 y1 y2封装到更高级别的抽象中(例如，记录)，或者至少适当地命名它们。(x, y, width, height)将大大提高您的代码的易懂性。现在，您必须仔细分析代码是如何编写的，以确定您是否只使用坐标，或者x2和y2是否以某种方式表示矩形的宽度和高度。

使用更高级别的抽象也会减少传递参数的数量，所以我会自己做。

我可以用什么方法来增加绘图例程中使用的封装量，这样我就不会在画图例程中定义变量了？尤其是y2变量是令人恼火的。

按照上述步骤改进DrawRect的工作方式。至于在Paint中显示的变量，这取决于您希望在重构中走多远。在某种形式上，你所使用的逻辑必须存在。我确实同意，也许最好把它从绘图程序中拿出来。

就我个人而言，我会将有关定位的所有信息提取到描述整个布局的更高层次的结构中。我的绘图例程将收到给定的布局并显示在屏幕上。