First, I'll go on about how the code looks, since it displays the most obvious issues.
You have redundant comments all over the place.
// Rectangle drawing function
DrawRect makes it clear enough that you're drawing a rectangle in there. No need for the comment.
// Define Y2
y2 := TextHeight(fLinesText.strings[count])*2;
And in the case of:
// Define Y1,Y2
x2 := Width - Width div 20;
y1 := Width div 20;
it's not only redundant, but also misguiding, since you're not setting the value of Y2 anywhere in there.
Code should be self-documenting, comments should not explain what you're doing, at most they should explain why you're doing what it is that you're doing. Otherwise, they're just cluttering the file, requiring extra work to maintain and looking ugly.
You should work towards improving the names of your variables.
// Define Link Rectangle
Lrect := Rect(x1, y1, x2, y2*(i+1));
LinkRectangle or at least
LinkRect, you can also discard the comment above the assignment.
LinkRect := Rect(...); makes it clear enough that you're setting up a link rectangle.
clR are some awfully confusing names, and unless you look at what parameters are being passed to the function in an actual call, it's pretty hard to understand what they're supposed to do. Consider
bttn_dwn could become
button_down (if this is the style you decided to apply to what seem to be unsigned integer constants).
txts could become
count is more often used to represent the number of items in a particular collection. For a loop counter, consider using a more common and appropriate name. In your particular case, there should be no issue to rename
And so on...
There is some inconsistency in the way you use the casing for your names.
Lrect is a parameter like all the others, it's being cased differently.
You chose to use
Function while also using
procedure. This sort of stands out, consider changing
Procedure, depending on how you want to be consistent. Personally, I prefer to not start with an uppercase character. Mainly because I don't start the rest of the keywords with one either, and I see you've also done that.
You should try to keep these things consistent throughout your code. Choose a way to name your things, and keep it throughout your project.
You could improve the way your code behaves.
if fLinesText.Count = 0 then exit
could well become
if fLinesText.Count > 0 then.
However, this too is redundant because
for count := 0 to fLinesText.Count - 1 do begin will do nothing if
Count is less than 1.
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]
Brush.color := stateColor[bttn_off];
into a function that returns values of the type the
stateColor elements have. Then call it from
TOC_MenuPanel.Paint, and pass the result as parameter to
DrawRect. This will ease the burden caused by the many parameters on
DrawRect (you'll be able to get rid of
clickedRect). It will also remove the responsibility of knowing that there are chosen and clicked rectangles, from
In the case of:
Lrect := Rect(x1, y1, x2, y2 * (i + 1));
You seem to be altering the rectangle's size based on the rectangle's index in the menu. There is no reason for
DrawRect to even know that there's a menu and that the rectangle being drawn has an index in it. The raw value of
y2 is not used anywhere else, so consider passing
y2 * (i + 1) as parameter (from within
MenuPanel.Paint), and getting rid of
i as parameter. If you extract the color choosing bit in a separate function, and call it from
Paint, doing this too should be trivial.
Consider wrapping up
x1 x2 y1 y2 into some higher level abstraction (a record for example) or at least name them appropriately.
(x, y, width, height) would go a long way towards making your code more understandable. Right now you have to carefully analyze how the code is written to find out if you're working exclusively with coordinates or if
y2 somehow represent the width and height of the rectangle.
Using a higher level abstraction will also reduce the number of parameters you pass around, so I'd do that myself.
What methods could I use to increase the amount of encapsulation used in the drawing routine so I'm not defining variables in the Paint routine? The y2 variable in particular is irritating.
Follow the steps above to improve the way
DrawRect works. As for the variables showing up in
Paint, it depends on how far you want to go with your refactoring. In some form or another, the logic you used there has to exist. I do agree that it's probably better to take it out of the drawing routines.
Personally, I'd extract everything about positioning into some higher level structure that describes the whole layout. My drawing routines would receive the given layout and show it on the screen.