Making a square move counter-clockwise around the edge

This is so ugly code. There must be better way to write it, but I'm noob to AS3.

The goal is to make a square move at even speed counter-clockwise around the edge and on call reverseDirection();, changeColor();, or spin();.

I'm trying to understand Tweenlite vars onStart and onStartParams. I'm sure that it can be used to write this elegantly with only one TweenLite.to.

I use AS3, Starling and Tweenlite. This is the code I'm trying to simplify:

```
public class Square extends Sprite implements ISquare {

public const square:Quad = new Quad(100, 100);
private var tweeningDirection:String;

public function Square() {
}

public function draw():void{
addChild(square);

square.pivotX = square.width / 2;
square.pivotY = square.height / 2;
square.x = 50;
square.y = 50;
square.color = 0x4500FF;
tweeningDirection = new String;

down();
}

public function reverseDirection():void{
TweenLite.killTweensOf(square);

switch (tweeningDirection){
case "down":
stoppedDown();
break;
case "left":
stoppedLeft();
break;
case "up":
stoppedUp();
case "right":
stoppedRight();
break;
case "stoppedDown":
down();
break;
case "stoppedLeft":
left();
break;
case "stoppedUp":
up();
break;
case "stoppedRight":
right();
break;
case "rDown":
stoppedRDown();
break;
case "rLeft":
stoppedRLeft();
break;
case "rUp":
stoppedRUp();
break;
case "rRight":
stoppedRRight();
break;
case "stoppedRDown":
rDown();
break;
case "stoppedRLeft":
rLeft();
break;
case "stoppedRUp":
rUp();
break;
case

After looking thoroughly at your code, I found a bug. Please be sure this is intended behavior, and if not, fix it.

In reverseDirection, you have this:

switch (tweeningDirection){
    case "down":
        stoppedDown();
        break;
    case "left":
        stoppedLeft();
        break;
    case "up":
        stoppedUp();//MISSED BREAK!
    case "right":
        stoppedRight();
        break;

And, as I pointed out with a comment in this code snippet, you've forgotten a break. This will cause both stoppedUp and stoppedRight to be called when reversing during the up tween, potentially causing a crash again like you experienced before.

Magic numbers: replace all literal numerics that are not 0 or 1 with a constant so you know what it means. I think I can see a BORDER_SIZE in there, with all those 50's. Or do they mean something else? I'm also seeing / 200 as a speed multiplier of sorts.

Now, for the actual question.

For each tween you use, you need a speed, a target, and another tween that will follow it. Additionally, you'll need to have a reverse tween.

So I'd write a class named ReversableTween containing something like this:

getTweenSpeed(objectToTween:?):Number //Square is Quad, but maybe we want a superclass of that
createTweenVars(onComplete:Function):Object //creates the entire third argument for TweenLite.to, assuming the Linear.easeNone needs to be added always
getReverseTween():ReversableTween //gets the r version
var attribute:String;//holds x or y, but could hold wacky stuff like width or height
var targetValue:Number;//holds the target value for the attribute (attribute = x, targetValue = 50)
var reverseTween:ReversableTween;
var nextTween:ReversableTween;
//constructor, getters, setters, whatever

Then I'd set things up like so...

var LEFT_SIDE:Number = BORDER_SIZE;
var RIGHT_SIDE:Number = stage.stageWidth - BORDER_SIZE;
var TOP_SIDE:Number = BORDER_SIZE;
var BOTTOM_SIDE:Number = stage.stageHeight - BORDER_SIZE;

Creating the objects...

var downTween:ReversableTween = new ReversableTween("y", BOTTOM_SIDE);
var stoppedDownTween:ReversableTween = new ReversableTween("y", TOP_SIDE);
//many more

Apparently, it seems the constructor takes attribute:String, value:Number. Because that's the information that we can provide directly.

Linking them via functions...

setReverse(downTween, stoppedDownTween);//sets a's reverse tween to be b and vice versa
//more invocations, per pair
setNext([downTween, rightTween, upTween, leftTween]);//1.next = 2, 2.next is 3... 4.next is (5 doesn't exist, so first element) 1
//more invocations, per circle (normal, reverse)
stoppedDownTween.setNextTween(rRightTween);
//the stopped versions need to be manually linked, 
//since they switch between circle 1 (normal) and circle 2 (reverse)

And then I'd have 1 onComplete function, which looks at the current tween and grabs the next...

public function onComplete(e:Event):void {
    currentTween = currentTween.getNext();
    TweenLite.to(square, currentTween.getTweenSpeed(square), currentTween.createTweenVars(this.onComplete));
}

I'd also get rid of the switch, instead only having to do

public function reverseDirection():void {
    TweenLite.killTweensOf(square);
    currentTween = currentTween.getReverseTween();
    TweenLite.to(square, currentTween.getTweenSpeed(square), currentTween.createTweenVars(this.onComplete));
}

Now, I just had to copy paste something, which is an indication that it could be better. So lets alter onComplete and reverseDirection to use startTween instead...
Also, see that I moved the killTweensOf - we only want 1 tween active at all times anyway.

public function onComplete(e:Event):void {
    currentTween = currentTween.getNext();
    startTween(currentTween);
}

public function reverseDirection():void {
    currentTween = currentTween.getReverseTween();
    startTween(currentTween);
}

public function startTween(tween:ReversableTween):void {
    TweenLite.killTweensOf(square);
    TweenLite.to(square, currentTween.getTweenSpeed(square), currentTween.createTweenVars(this.onComplete));
}

Lastly, lets fix the naming, it just doesn't feel right for me...

public function nextTween(e:Event = null):void {
    currentTween = currentTween.getNext();
    startTween(currentTween);
}

public function reverseTween():void {
    currentTween = currentTween.getReverseTween();
    startTween(currentTween);
}

public function startTween(tween:ReversableTween):void {
    TweenLite.killTweensOf(square);
    TweenLite.to(square, currentTween.getTweenSpeed(square), currentTween.createTweenVars(this.nextTween));
}

I added default null to onComplete so I can call it without having to pass an argument. I then renamed it to nextTween (since that's what it does!). I also renamed reverseDirection to reverseTween, so that I have 3 neatly named functions that look the same: nextTween, reverseTween and `startT

StackExchange Code Review Q#57665, answer score: 12