How to properly Optimize

Question

So I`ve manage to create a simple fla and manage to upload it to http://wonderfl.net/c/unNZ so if anyone likes to help me he it would be easy.(I did think of posting the code here, but i dont think ppl would read 300+ lines of code and after that help me without knowing visually what i have done so far and what i wanted )

So long story short -> the code in the link does this:

-Checks to see if the cell of the Grid we want to put our object is free - if it is, we can place the box in it.

-Checks to see if the cell that is left of the current object(Box) is occupied - if it is, Check to see if the object in it is 1 less then the current - if it is join them together / if not do nothing (just place it)

-Combine complex objects (like add a object that has 2 boxes(or more) in it to another object that has 2 or more boxes in it ).

Yet the way i have done the coding is not efficient at all.There are many things that i think are not maybe needed, but yet again i couldnt think of a better solution that would in the end work and show what Im aiming for.

The problem is not that im not satisfied with the code i have so far, the problem is that when I add the Check to see if RIGHT / TOP / BOTTOM cell logic (currently i have only the check LEFT cell logic )it would get quite absurd/chaotic and even now i dont know if i would manage to do it.

So is there someone who would help me do this.

Answer 1

What you want to do at this point of your code is refactor.

Basically, you need to take your chucks of code, and break them up into smaller functions. Ideally, no function would be more than about 5 lines of code. Because otherwise, your function is breaking the single responsibility rule and doing too much.

There are two ways of refactoring at this point. The harder longer way, or the easier even longer way. The harder longer way, is to manually attempt to break up your code into smaller functions. See what elements are common, and refactor those out. For example this code:

for( var k:int = 0; k <= ifl; k++ )
                {
                    Grid[ numbers[1] ][ (numbers[0] - k ) ].alpha = 1;
                    Grid[ numbers[1] ][ (numbers[0] - k ) ].circle.visible = true;
                    Grid[ numbers[1] ][ (numbers[0] - k ) ].isOccupied = false;
                    trace( " ++++++!!!! grid was occupied" + numbers[1] + "_" + (numbers[0] - k ) ) ;
                }

Would become this code:

 private function clearOccupiedSpaces(ifl:Array):void
{
      for( var i:int = 0; i <= ifl; i++ )
        {
              Grid[ numbers[1] ][ (numbers[0] - i ) ].alpha = 1;
              Grid[ numbers[1] ][ (numbers[0] - i ) ].circle.visible = true;
              Grid[ numbers[1] ][ (numbers[0] - i ) ].isOccupied = false;
              trace( " ++++++!!!! grid was occupied" + numbers[1] + "_" + (numbers[0] - i ) ) ;
        }
}


            if( currentObj.itemsFromLeft > 0 )
            {
                trace("zzzzzzz");
                //when we start to drag it (we remove it from the cell) we restore all the properties to the LEFT cell
                var ifl:Number = currentObj.itemsFromLeft; // total left elements
                clearOccupiedSpaces(ifl);
            }  

Once you break up everything into smaller functions, your code will look less chaotic and you will be able to more easily find patterns,and manage the code. You will also need less comments, since your functions will be clearly labeled as to what they are doing.

The second and better option, is to refactor the code using TDD or Test Driven Design. This will allow you to solve your algorithms by writing tests. You will naturally end up with smaller functions, and be able to know if some change accidentally broke a use case. For example, you have LEFT check working now. But what if when you write your RIGHT check, you break something in LEFT? You won't really know that until it's too late, unless you use Unit tests.

An example of a unit test would be something like this:

public function testLeftCheck_nothingToTheLeft_placeObject():void {
           var box222:Box222 = new Box222();
           box222.itemsFromLeft = null;
           placeObject(box222);
           assertTrue(box222.width == SINGLE_BOX;);
    }

    public function testLeftCheck_somethingToTheLeft_combineObject():void {
          var box222:Box222 = new Box222();
           box222.itemsFromLeft = [new Box222()];
           placeObject(box222);
           assertTrue(box222.width == TWO_BOX_WIDTH;);   
    }