I’m currently implementing a drag and drop sorting feature for a tree view in a Windows Forms application. We’re using Infragistics UI controls, and their UltraTree doesn’t have drag and drop functionality out of the box. They do however have a sample in their SDK which explains how to implement it. Here’s an excerpt of that sample code – a method which determines where to draw a marker which tells the user where in the tree he’s about to drop (ie before or after the node the mouse pointer hovering):
public void SetDropHighlightNode(UltraTreeNode Node, Point PointInTreeCoords )
{
//The distance from the edge of the node used to
//determine whether to drop Above, Below, or On a node
int DistanceFromEdge;
//The new DropLinePosition
DropLinePositionEnum NewDropLinePosition;
DistanceFromEdge = mvarEdgeSensitivity;
//Check to see if DistanceFromEdge is 0
if (DistanceFromEdge == 0)
{
//It is, so we use the default value - one third.
DistanceFromEdge = Node.Bounds.Height / 3;
}
//Determine which part of the node the point is in
if (PointInTreeCoords.Y < (Node.Bounds.Top + DistanceFromEdge))
{
//Point is in the top of the node
NewDropLinePosition = DropLinePositionEnum.AboveNode;
}
else
{
if (PointInTreeCoords.Y > ((Node.Bounds.Bottom - DistanceFromEdge) - 1))
{
//Point is in the bottom of the node
NewDropLinePosition = DropLinePositionEnum.BelowNode;
}
else
{
//Point is in the middle of the node
NewDropLinePosition = DropLinePositionEnum.OnNode;
}
}
//Now that we have the new DropLinePosition, call the
//real proc to get things rolling
SetDropHighlightNode(Node, NewDropLinePosition);
}
Disregarding the clearly redundant comments (NewDropLinePosition is “the new DropLinePosition”? phew, I’d never have guessed!), there are some that you might think deserve to stay – the ones that tell you what the if/else conditions are checking for, for instance.
However, take a look at my refactored version of this method. It has ZERO comments – yet I would argue that it is much easier to understand:
public void HighlightPotentialDropNode(
UltraTreeNode potentialDropNode,
Point dropLocationRelativeToTree)
{
DropLinePositionEnum newDropLinePosition = DropLinePositionEnum.OnNode;
if (PointIsAboveNode(potentialDropNode, dropLocationRelativeToTree))
{
newDropLinePosition = DropLinePositionEnum.AboveNode;
}
else if (PointIsBelowNode(potentialDropNode, dropLocationRelativeToTree))
{
newDropLinePosition = DropLinePositionEnum.BelowNode;
}
DrawDropLinePosition(potentialDropNode, newDropLinePosition);
}
private bool PointIsAboveNode(UltraTreeNode node, Point pointInTreeCoords)
{
return pointInTreeCoords.Y < (node.Bounds.Top + EdgeSensitivity);
}
private bool PointIsBelowNode(UltraTreeNode node, Point pointInTreeCoords)
{
return pointInTreeCoords.Y > ((node.Bounds.Bottom - EdgeSensitivity) - 1);
}
Apart from renaming stuff to give them more meaningful names, notice particularly how I’ve used extract method on the predicates of the if statements, giving them names which clearly state what the conditions are. This has an important effect: All the code in this method is now at the same level of abstraction – and this makes it much more readable.
Extract method is probably the simplest and most effective refactoring tool in your arsenal. Use it lavishly! If you want to learn more about writing clean functions, I suggest you take an hour to watch Robert C. Martin’s great “Clean Code: Functions” talk – here’s a recording from NDC 2009 (see more NDC recordings here).
In my last post, I refactored some horribly commented code into the following:
public void HighlightPotentialDropNode(
UltraTreeNode potentialDropNode,
Point dropLocationRelativeToTree)
{
DropLinePositionEnum newDropLinePosition = DropLinePositionEnum.OnNode;
if (PointIsAboveNode(potentialDropNode, dropLocationRelativeToTree))
{
newDropLinePosition = DropLinePositionEnum.AboveNode;
}
else if (PointIsBelowNode(potentialDropNode, dropLocationRelativeToTree))
{
newDropLinePosition = DropLinePositionEnum.BelowNode;
}
DrawDropLinePosition(potentialDropNode, newDropLinePosition);
}
private bool PointIsAboveNode(UltraTreeNode node, Point pointInTreeCoords)
{
return pointInTreeCoords.Y < (node.Bounds.Top + EdgeSensitivity);
}
private bool PointIsBelowNode(UltraTreeNode node, Point pointInTreeCoords)
{
return pointInTreeCoords.Y > ((node.Bounds.Bottom - EdgeSensitivity) - 1);
}
I was a bit quick to publish that post – because 5 minutes later I discovered that I wasn’t quite done cleaning up this method... Can you tell what’s wrong with it? It’s breaking the single responsibility principle.
Let’s do one more ‘extract method’ refactoring on it, putting the logic of calculating the DropLinePosition in a separate method:
public void HighlightPotentialDropNode(
UltraTreeNode potentialDropNode,
Point dropLocationRelativeToTree )
{
DropLinePosition dropLinePosition =
CalculateDropLinePosition(potentialDropNode, dropLocationRelativeToTree);
DrawDropLinePosition(potentialDropNode, dropLinePosition);
}
private DropLinePosition CalculateDropLinePosition(
UltraTreeNode potentialDropNode,
Point dropLocationRelativeToTree)
{
DropLinePosition dropLinePosition = DropLinePosition.OnNode;
if (DropLocationIsAboveNode(potentialDropNode, dropLocationRelativeToTree))
{
dropLinePosition = DropLinePosition.AboveNode;
}
else if (DropLocationIsBelowNode(potentialDropNode, dropLocationRelativeToTree))
{
dropLinePosition = DropLinePosition.BelowNode;
}
return dropLinePosition;
}
private bool DropLocationIsAboveNode(UltraTreeNode node, Point dropLocationRelativeToTree)
{
return dropLocationRelativeToTree.Y < (node.Bounds.Top + EdgeSensitivity);
}
private bool DropLocationIsBelowNode(UltraTreeNode node, Point dropLocationRelativeToTRee)
{
return dropLocationRelativeToTRee.Y > ((node.Bounds.Bottom - EdgeSensitivity) - 1);
}
272
被折叠的 条评论
为什么被折叠?
到【灌水乐园】发言
