kuwamoto.org

We’re now ready for our final step. We started with something that looked like this, and we are going to end up with something that looks like this. As you can see, the layout of the container is assignable at runtime.

After step 2, there was very little refactoring to do. I removed the DragTile class once and for all, and I added styles back in. The way that the Flex styling mechanism works, you can implement styles for non UIComponents, and that’s what I did. Each Layout object can have its own styles.

I created another concrete layout called CircleLayout to make sure that the layout was actually assignable at runtime. This exposed a few bugs, which I fixed. I factored out the common code between TileLayout and CircleLayout into a common concrete superclass, called Layout. In the end, the inheritance hierarchy for the layout managers looks like this:

I also “fixed” some nitpicky things along the way. I’ve always thought that the drag/drop feedback was a bit confusing, so I changed the behavior in a subtle way (see if you can spot it!).

In the end, the true test of whether this refactoring worked is to see how easy it is to add a new layout. Whereas the original DragTile code was 600+ lines of code, the CircleLayout code is only about 100 lines of code, and all of it pertains to geometry. There is virtually no code in there to manage renderers or animators or anything like that.

public class CircleLayout extends Layout
{
// ILayout interface		
override public function getMeasuredSize():Point
{
	return getMaxSize();
}

override public function findItemAt(px:Number, py:Number, seamAligned:Boolean):Number
{
	// Can't execute this if we aren't attached to a container.
	if (!container || container.renderers.length == 0)
		return NaN;
		
	// Get the radius and center of the circle.
	var radius : Number = Math.min(unscaledContainerWidth, unscaledContainerHeight) / 2;
	var hCenter : Number = unscaledContainerWidth / 2;
	var vCenter : Number = unscaledContainerHeight / 2;
	
	var angle : Number = Math.atan2(py-vCenter, px-hCenter);
	if (angle < 0)
		angle += 2 * Math.PI;

	// figure out the closest "item" by working backwards from the angle to the index, using floating point math.
	var result : Number = container.renderers.length * angle / (2 * Math.PI);

	// depending on whether this is seam aligned, do a ceil or round.			
	result = (seamAligned) ? Math.ceil(result) : Math.round(result);
	
	// do a modulo op to make sure that this is within [0, length-1]. Modulo is the correct
	// operator in this case because this is a circle.
	result %= container.renderers.length;
	return result;
}

override public function generateLayout():void
{
	// Get the radius and center of the circle.
	var radius : Number = Math.min(unscaledContainerWidth, unscaledContainerHeight) / 2;
	var hCenter : Number = unscaledContainerWidth / 2;
	var vCenter : Number = unscaledContainerHeight / 2;

	// Find the max item size.
	var maxSize : Point = getMaxSize();
	var max : Number = Math.max(maxSize.x, maxSize.y);
	
	// Inset the radius by the max size.
	radius -= max;

	// Loop through the items and position them.
	var length : int = container.renderers.length;
	for (var idx:int = 0; idx < length; idx++)
	{
		var renderer:IUIComponent = container.renderers[idx];
		var target:LayoutTarget = animator.targetFor(renderer);//targets[idx];
		
		// evenly space each item over 2*pi radians.
		var angle : Number = (2 * Math.PI) * idx / length;
		
		// position items on a circle.
		target.scaleX = target.scaleY = 1;
		target.item = renderer;
		target.unscaledWidth = renderer.getExplicitOrMeasuredWidth();
		target.unscaledHeight = renderer.getExplicitOrMeasuredHeight();
		target.x = hCenter + radius * Math.cos(angle) - target.unscaledWidth/2;
		target.y = vCenter + radius * Math.sin(angle) - target.unscaledHeight/2;
		target.animate = true;					
	}
	
	// If there is more than one item, and if there is a drag target, nudge the items next to the drag target
	if (length > 1 && container.dragTargetIndex >= 0 && container.dragTargetIndex < length)
	{
		// Find the items to the left and right of the target.
		var leftIndex : int = (container.dragTargetIndex + length - 1) % length;
		var rightIndex : int = (leftIndex + 1) % length;
		
		var leftTarget : LayoutTarget = animator.targetFor(container.renderers[leftIndex]);
		var rightTarget : LayoutTarget = animator.targetFor(container.renderers[rightIndex]);
		
		// exaggerate the difference between the two targets by a factor of maxSize/2.
		var dx : Number = rightTarget.x - leftTarget.x;
		var dy : Number = rightTarget.y - leftTarget.y;
		var distance : Number = Math.sqrt( dx*dx + dy*dy );
		
		leftTarget.x -= dx / distance * max/2;
		leftTarget.y -= dy / distance * max/2;
		rightTarget.x += dx / distance * max/2;
		rightTarget.y += dy / distance * max/2;
	}
}

protected function getMaxSize() : Point
{
	// Can't execute this if we aren't attached to a container.
	if (!container)
		return new Point(0, 0);
		
	// Find the max item size.
	var maxWidth : Number = 0;
	var maxHeight : Number = 0;
	
	if(container.renderers.length > 0)
	{
		for(var i:int=0;i<container.renderers.length;i++)
		{
			var itemRenderer:IUIComponent = container.renderers[i];
			maxWidth = Math.ceil(Math.max(maxWidth,itemRenderer.getExplicitOrMeasuredWidth()));
			maxHeight = Math.ceil(Math.max(maxHeight,itemRenderer.getExplicitOrMeasuredHeight()));
		}
	}

	return new Point(maxWidth, maxHeight);
}

Could this be improved? Sure. There are still linkages between the layout and the container that we should probably get rid of. But for now, I think I’m going to stop. Maybe I’ll refactor the rest of it away later. :-)

Code for the final version can be found here.

In Step 1, we created a superclass to hold the common logic for all flexible layouts. Now, let’s change the model from inheritance to composition. My goal in this step was to move all DragTile functionality into a separate helper class, leaving DragTile as just an empty shell.

Until now, we have not had to worry about the contract between the container and the layout manager. All we did in step 1 was decide which methods belonged in the superclass and the subclass on a somewhat haphazard basis.

Now that we want to move this into a separate helper object, we need to ask ourselves how the main object (the container) needs to communicate with the helper object. A good starting point is to look at the members that the DragTile subclass defines.

All of the properties (green dots) seem to be specific to DragTile. Meanwhile, three member functions at the bottom look truly general: measure(), findItemAt(), and generateLayout(). Let’s use these as our initial interface for ILayout.

Because measure() is a method defined on IUIComponent, It doesn’t make sense to just move it to a non-UIComponent object. Instead, I decided to define a helper function, getMeasuredSize(), that we call from the measure() function. And thinking through some of the other complexities that might arise, I added an attach() and detach() method so that the layout manager could do some initialization and cleanup whenever it was used. The interface now looks like this:

I now had to make this “real” by creating a layout property of FlexibleContainer and creating a concrete implementation of ILayout called TileLayout.

How do we get from where we are in step 1 to where we need to get to in step 2? In step 1, DragTile is a subclass of FlexibleContainer. After step 2, we know that there won’t be a class called “DragTile” anymore.

This seems like a hard change to make incrementally, and the whole point of refactoring is to do these changes in small, incremental steps. Which brings me to the next tip.

Tip 5: If needed, build temporary scaffolding to make sure your code continues to “work” as you refactor.

In this case, I used both inheritance and delegation while I was in the middle of the refactoring. In other words, I wrote a little bit of scaffolding code to keep the DragTile class around even after I had created the TileLayout class, and slowly moved code over from one to the other.

For those of you who are not familiar with the ideas behind refactoring, this writing of temporary code might seem kind of insane. However, once you get used to it, you will never go back. Big changes are much easier to do successfully in small, bite-sized pieces. Sometimes this means writing a few lines of shim code to hold things up as you change things incrementally.

The next step was to move all the functionality out of the DragTile subclass. During this phase, I didn’t move entire methods. Instead, I dissected methods line by line and moved the correct functionality over to the appropriate class using the following simple rules:

• If the logic contained in the line of code was “general”, it move it to the FlexibleContainer superclass
• If the logic contained in the line is specific to tiles, move it to the “TileLayout” class

This process worked fine for most of the logic, but the styles were a different matter. TileLayout is not a UIComponent, so it doesn’t participate in the CSS system. Meanwhile, the styles that DragTile defines are specific to tiles (vGap, hGap, etc). While it is possible to have things that are not UIComponents participate in CSS, it takes a lot of work, and I didn’t feel like tackling it right then.

What I decided to do was to turn off the style functionality temporarily. This brings us to the next tip:

Tip 6: When temporarily disabling functionality during a big refactor, be sure to do so in a way that preserves information, ideally through stub functions.

One way to turn off the CSS functionality would have been to comment out the lines of code that refer to CSS and restore them later. The problem with this approach is that it is difficult to make sure that the the code inside your comments continues to be “correct” as you move your code around.

To help make sure that the CSS knowledge that was embedded in Ely’s code didn’t get lost, I created a stub getStyle() function that always returned NaN. This meant that all of the CSS logic in Ely’s code was still there and still functioning, but it was attached to a CSS style sheet where nothing was ever defined.

By keeping Ely’s code alive, it was easy to make sure that I was making the right edits to it as I refactored. As I moved code around, the compiler would remind me if I was doing something terribly wrong. This doesn’t prevent all coding errors, but it helps prevent many of them.

After all this, I was able to move all of the code from the DragTile code into one of the other two classes. The final version of the DragTile class looked like this:

public class DragTile extends FlexibleContainer
{
	public function DragTile()
	{
		super();
		
		layout = new TileLayout();
	}
}

We are now (obviously) at the point where we can get rid of the DragTile object. Code for step 2 can be found here

In the intro, we talked about the problem we want to tackle, which is to refactor DragTile to be more flexible. If you haven’t read it yet, you might want to check it out.

Back? Ok. Now, let’s look at some code.

Step 0 – the starting point

The original files for my investigation can be downloaded from Ely’s site, or you can get my slightly munged version here.

Step 1 – pull up the FlexibleContainer superclass

Even though we said we were going to use composition instead of inheritance to separate these classes, we do this as a first step because it is easier than pulling out a helper class.

Tip 3: Always refactor in small steps that leave the external behavior unchanged

The basic procedure is to create a superclass (which I called “FlexibleContainer”) and to walk through the DragTile code method by method and property by property to move things to the superclass if it seems appropriate.

I started by moving all of the local variables that seemed “general” to the superclass. This included the _items array, the renderers array, and so forth. The variables that seemed specific to the Tile layout were left in the DragTile class.

Because most of these variables were private, moving them to the superclass caused a lot of compile errors, most of which I fixed by moving the appropriate methods to the superclass.

In one case (dragTargetIndex) I needed to create a protected accessor so that the subclass could get at this information. If you want to be a stickler about it, making a private member protected requires some thought. Is this the right way for a subclass to get information from its superclass? In this case, it’s probably not the right thing. The “right” thing is probably to pass the drag information as a parameter during the drag operation. This brings us to the next tip:

Tip 4: When refactoring, don’t try to make it “perfect”. Just strive to incrementally improve the code each time you touch it.

Creating a protected accessor here was the quickest way to separate these two classes without overcomplicating the design. We can come back and fix this later after the dust has settled.

More »

I love Ely’s DragTile component (demo). However, I have always wished that it were easier to extend its behavior to encompass different layouts. To me, that immediately suggests a (big) refactoring. For those of you who have never done refactoring or who don’t refactor as often as you should, I thought it might be fun to walk through this step by step.

What is refactoring?

Refactoring is the process of incrementally changing the structure of your code without changing the outward functionality. It may seem silly to focus on not changing the functionality, but it is actually pretty important.

The idea is that there are two distinct phases of work: coding and refactoring. During coding, you add new functionality. During refactoring, you restructure your code while making sure that it continues to work properly. Not changing functionality during refactoring gives you a strong reference point as you make lots of iterative changes to the structure. You know your refactoring was successful if the new code still does what the old code did.

Refactoring for better reuse

Usually, you refactor code so that it can be more flexible or more reusable that it currently is. Sometimes, the change that is required is obvious. In other cases, you have to think a little before figuring out how to transform your code.

In this particular case, we want to make it easier to change layouts. Looking at the DragTile code, there is quite a bit that would be the same no matter what the layout is, and relatively little that needs to change with each new layout algorithm. Because the layout algorithm is something that is likely to change in various use cases, it is probably a good idea to isolate this functionality into a separate class.

Tip 1: Cleanly separate out the code that you think will need to change often into a separate class.

The most obvious way to divide up the DragTile layout code from the rest of the DragTile code is through inheritance:

Splitting things up in this way allows us to create a new subclass with a different layout (say, a circle) while reusing the logic that might be common to both, such as the code to talk to item renderers and do animation.

The other main way to split things up is through composition:

In this case, the container delegates to another object in order to do layout. There are a number of advantages to this approach:

1. Using delegates often makes it possible to modify behavior at runtime. After we are finished with this particular refactoring, we should be able to change the layout of one of our containers without reparenting. Woohoo!!
2. Decomposing larger classes into smaller classes can make it easier to evolve different parts of the system separately as needs change. For example, let’s say we later decide to split up FlexibleContainer into two classes: one that is lightweight for easy download, and another that is robust, to handle caching, localization, etc. Should “DragTile” inherit from the light one or the robust one? If layout is handled through delegation, you may not need to choose.
3. Composition often allows for greater decoupling. For example, let’s say you are building a photo organizer module. With the inheritance approach above, it would be difficult to create the module in such a way that the photo module knows absolutely nothing about the layout. With the composition approach, you could provide a default layout (say, Tile), while allowing the user of the module to pass in a different layout if needed.

All of this leads to the next tip, which is:

Tip 2: Think hard before using inheritance. Composition is almost always a better way to separate out the flexible part of a class from the invariant part.

So now we have a general idea of what we want to separate out, and how we want to do it. We’ll start digging into actual code in the next post.