More efficient way to write this Actionscript 3 class?

Question

I wrote one of my first classes in Actionscript 3 and I want someone versed in AS3 to help me fine tune my class. I am sure there are things that could really help with efficiency and I love learning about Object Oriented "Programming" so please take a look and let me know what you think.

Project background; This project is a card game that reads all the cards information from an XML file so the client can easily add, edit, or delete card information. The user will drag an Activity card from one of the six stacks and place it on top of the Event card. The six stacks will have a card face up so the user can see the information and match the best card to the Event card (ie ... an Event card may read "Burning auto wreck with a person inside" and the Activity cards may read "Pull Victim" or "Put out Flames" or etc .....

When the user makes the drop the Activity card on the Event Card it goes thru a point check and determines if it was a good drop and assigns points accordingly.

After the drop and points happen the next user clicks "Next >>" and the deck reshuffles and the process starts all over again until a certain amount of points occur.

Hopefully that made sense and you can get sone kind of idea on how it works, if not I can elaborate more if needed.

Document Class (CardGame.as)

package library
{
import flash.display.*;
import flash.events.*;
import flash.net.*;

public class CardGame extends Sprite
{
    /*
    *   variables
    */
    private var _xml:XML;
    private var _loader:URLLoader;
    private var _cardArray:Array;
    private var _activity:ActivityCard;
    private var _event:EventCard;
    private var _randomCardArray:Array;
    /*
    *   contructor
    */
    public function CardGame():void
    {
        _loader                          = new URLLoader();             
        _loader.addEventListener(Event.COMPLETE,buildArray);             
        _loader.load(new URLRequest("library/card.xml"));
        start_btn.addEventListener(MouseEvent.CLICK,buildDeck);
        restart_btn.addEventListener(MouseEvent.CLICK,removeDeck);
    }
    /*
    *   metohods
    */
    public function buildArray(e:Event):Array
    {
        _cardArray                       = new Array();  
        _xml                             = new XML(_loader.data);
        for(var i=0;i<_xml.card.length();i++)
        {
            if(_xml.card.face[i]== "activity")
            {
                _cardArray.push({id:_xml.card.id[i],face:_xml.card.face[i],category:_xml.card.category[i],point:_xml.card.point[i],value:_xml.card.value[i]});
            }
            else if(_xml.card.face[i] == "event")
            {
                _cardArray.push({id:_xml.card.id[i],face:_xml.card.face[i],category:_xml.card.category[i],point:_xml.card.point[i],value:_xml.card.value[i]});
            }
            else
            {
                trace("no card found");
            }
        }
        return _cardArray;
    }
    private function randomizeArray():Array
    {
        //trace(buildArray(null));
        _randomCardArray                  = new Array();
        do 
        {
            var cardItem                  = int(Math.random()*_cardArray.length);
            _randomCardArray.push(_cardArray[cardItem]);
            _cardArray.splice(cardItem,1);
        } 
        while (_cardArray.length > 0);
        return _randomCardArray;
    }
    public function buildDeck(e:Event):void
    {
        var _finalCardArray               = randomizeArray();
        //trace(_finalCardArray);
        for(var i=0;i<_finalCardArray.length;i++)
        {
            if(_finalCardArray[i].face == "activity")
            {
                _activity                 = new ActivityCard();
                _activity.x               = 200;
                _activity.y               = 150;
                _activity.buttonMode      = true;
                _activity.mouseChildren   = false;
                _activity.name            = "card"+i;
                _activity.id_txt.text     = _finalCardArray[i].id;
                _activity.value_txt.text  = _finalCardArray[i].value;
                _activity.addEventListener(MouseEvent.MOUSE_DOWN,dragStart);
                _activity.addEventListener(MouseEvent.MOUSE_UP,dragStop);
                addChild(_activity);
            }
            else if(_finalCardArray[i].face == "event")
            {
                _event                    = new EventCard();
                _event.x                  = stage.stageWidth/2;
                _event.y                  = 550;
                _event.mouseChildren      = false;
                _event.name               = "card"+i;
                _event.id_txt.text        = _finalCardArray[i].id;
                _event.value_txt.text     = _finalCardArray[i].value;
                addChild(_event);
            }
            else
            {
                trace("no cards");
            }
        }
    }
    public function dragStart(e:Event):void
    {
        _activity                           = e.target as ActivityCard;     
        _activity.parent.setChildIndex(_activity, numChildren - 1);     
        _activity.startDrag(false);
    }
    public function dragStop(e:Event):void
    {
        this.stopDrag();
    }
    private function removeDeck(e:Event):void
    {
        if(_cardArray.length == 0)
        {
            for(var i=0;i<106;i++)
            {
                removeChild(getChildByName("card"+i));
            }
            buildArray(null);
        }
        buildDeck(null);
    }
}
}

Thanks in advance :-)

Answer 1

There are a few areas that can be improved but I guess we need some more context as to how this is laid out. My comments and questions below:

1 - In ActionScript, since you can modify the contents of a collection inside a loop querying the length of the collection at each iteration is a small performance hit. So:

for(var i=0;i<_xml.card.length();i++)

should really be

var xmlLength:int = _xml.card.length();
for(var i=0;i<xmlLength;i++)

2 - To help with clarity you want to use strongly typed objects wherever possible. For example when you are storing each event or activity in the _cardArray consider having a class which just holds the data for those objects. This helps with runtime performance as well as other developers understanding not only the contents of the array but the data type of each property in the object. For example:

_cardArray.push({id:_xml.card.id[i],face:_xml.card.face[i],category:_xml.card.category[i],point:_xml.card.point[i],value:_xml.card.value[i]});

could instead be

_cardArray.push(new ActivityVO(_xml.card.id[i], _xml.card.face[i], _xml.card.category[i], _xml.card.point[i], _xml.card.value[i]));

where ActivityVO is(I'm just assuming the type of these properties):

public class ActivityVO {

    public var id:int;
    public var face:String;
    public var category:String;
    public var point:int;
    public var value:int;

    public function ActivityVO(_id:int, _face:String, _category:String, _point:int, _value:int){
        id = _id;
        face = _face;
        category = _category;
        point = _point;
        value = _value;
    }
}

3 - How does buildArray work right now? I see it is executed onComplete of the loader but it returns the array. If buildArray is called by something else(I see its public) then all of this logic will be executed twice. Where does this get returned to? If this is the document class what is calling this public method? randomizeArray seems to suffer from the same problem. I'm not sure where you are returning this array to.

4 - Think about having a new class which manages the deck of cards. Methods like randomizeArray and buildDeck could be within this class and reduce the clutter of your current class.

5 - The removeDeck method doesn't feel right. You should always try to avoid for loops with hard coded breaking values (106). Arguably a better way to go about the whole loop would be to maintain an array of all the cards on the table, then when it comes time to remove them they can all be cleaned up by looping through the array.

These comments aren't really in any particular order, I put them together as I was reading through your code. Let me know if you have any questions.