PDA

View Full Version : Care To Take A Look?


AdmiralAkbar
03-22-2011, 02:21 AM
I'm new to AS3 and OOP programming, and trying to wrap my mind around the concepts has not been easy. I don't expect anyone to meticulously go through each line of my code, but if you'd care to take a quick glance and see if any big no no's immediately stick out to you I'd appreciate it.

This is my program's main class:

package com.nwstudios.photo {

import flash.display.*;
import flash.events.*;
import flash.net.*;
import flash.utils.*;

public class PhotoManager extends MovieClip {

var photos:XML; // XML document containing info about photos

// Used To Display Thumbnails /////

var whichRow:int = 0; // Which row are we on?
var whichColumn:int = 0; // Which column?
var numberOfColumns:int = 5; // Total number of columns

var leftMargin:int = 50; // Left margin
var topMargin:int = 20; // Top Margin

var xPos:int = leftMargin; // Position of
var yPos:int = topMargin; // thumbnail on screen

var xSpace:int = 120; // Spacing between
var ySpace:int = 80; // thumbnails

var images:Array = new Array(); // Fill it with Thumbnail instances

//////////////////////////////////////////////////////////////////////////////////

function PhotoManager ():void {
loadXML("photos.xml");
}

function XMLIsReady (e:Event):void {
photos = new XML(e.target.data); // Load XML Document
loadThumbnails(); // Load photos described in XML Document
}

function loadThumbnails ():void {
for(var i:int = 0; i < photos.photo.length(); i++) { // Iterate through photos
images[i] = new Thumbnail(i, photos.photo.index[i], xPos, yPos); //create image object
addChild(images[i].getThumbnail()); //Display thumbnails
getXY(); // Get/set X and Y for next thumbnail
}

function getXY ():void {
if (whichColumn != numberOfColumns) { // Not on the last column
whichColumn++; // so go to the next one
}
else { // We are on the last column
whichColumn = 0; // so go back to the first one
whichRow++; // and go to the next row
}
xPos = leftMargin + (whichColumn * xSpace); // New x
yPos = topMargin + (whichRow * ySpace); // and y
}
}

//Load XML File
function loadXML (url:String):void {
var txtLoader:URLLoader = new URLLoader();
txtLoader.addEventListener(Event.COMPLETE, XMLIsReady);
txtLoader.load(new URLRequest(url));
}
}
}

I haven't included the code for the Thumbnail class because it's almost as cluttered as my desk and you don't want to see that mess. I'm curious to see if anyone has any input.

tadster
03-22-2011, 02:41 AM
Looks pretty good, only very minor performance issues not oop, oop mainly is about how one uses multiple classes together.

For instance you could have a separate class that does nothing more than load things, and the PhotoManager may extend or use that loader class.

But in your case that is not really needed.

As far as performance, you don't need to extend MovieClip unless you need a timeline, so just Sprite should be ok.

A good rule of thumb for performance is to limit the amount of 'new' calls used.
So the URLLoader can be instantiated in the constructor and just reused instead of making a new one for each loadXML call.

Oh, and the constructor should take a string as a param to pass to the first loadXML call

function PhotoManager (firstURLToLoad:String = "photos.xml"):void {
loadXML(firstURLToLoad);
}

AdmiralAkbar
03-22-2011, 03:22 AM
When I change this

public class PhotoManager extends MovieClip

to this

public class PhotoManager extends Sprite

I get this error

1180: Call to a possibly undefined method addFrameScript.

Also, this is my interpretation of what you told me. Is it close?

function PhotoManager (firstURL:String = "photos.xml"):void {
var txtLoader:URLLoader = new URLLoader();
txtLoader.addEventListener(Event.COMPLETE, XMLIsReady);
txtLoader.load(new URLRequest(firstURL));
}

And one more thing: I was trying to create a loader package that I could use to load all of my files, whether it's an image or an xml file. The only problem is that I want it to be available to other packages (like this one) since it seems pretty universal. Is that possible or even useful? Should I just include it in this package and copy it into any others that need to make use of it in the future?

I eventually want this to be a photo manager which will allow a user to log in, upload photos, crop thumbnails for them, change the order by dragging and dropping, etc., and organize them into photo albums which will correspond to different sections of their web site and be displayed. You might not know it by looking at my code, but I'm trying to learn the object oriented approach and the concepts are just starting to come into focus (although they're still blurry and difficult to think about!)

tadster
03-22-2011, 03:42 AM
Ok then, let's split it up a bit, and it seems you do need to extend MovieClip since you are using frames/the timeline.

package com.nwstudios.photo
{
import flash.events.Event;
import flash.events.EventDispatcher;
import flash.net.URLLoader;
import flash.net.URLRequest;


public class MasterLoader extends EventDispatcher
{
private var loader:URLLoader;

public function MasterLoader() {
loader = new URLLoader();

}

public function loadURL(url:String):void {
loader.addEventListener(Event.COMPLETE, LoadIsComplete);
loader.load(new URLRequest(url));
}

private function LoadIsComplete(e:Event):void {
loader.removeEventListener(Event.COMPLETE, LoadIsComplete);
this.dispatchEvent(e);
}
}
}




package com.nwstudios.photo {

import flash.display.*;
import flash.events.*;
import flash.net.*;
import flash.utils.*;

public class PhotoManager extends MovieClip {

public var photos:XML; // XML document containing info about photos

// Used To Display Thumbnails /////

private var whichRow:int = 0; // Which row are we on?
private var whichColumn:int = 0; // Which column?
private var numberOfColumns:int = 5; // Total number of columns

private var leftMargin:int = 50; // Left margin
private var topMargin:int = 20; // Top Margin

private var xPos:int = leftMargin; // Position of
private var yPos:int = topMargin; // thumbnail on screen

private var xSpace:int = 120; // Spacing between
private var ySpace:int = 80; // thumbnails

public var images:Array = new Array(); // Fill it with Thumbnail instances

private var masterLoader:MasterLoader;

//////////////////////////////////////////////////////////////////////////////////

public function PhotoManager(firstURLToLoad:String = "photos.xml"):void {
masterLoader = new MasterLoader();
masterLoader.addEventListener(Event.COMPLETE, XMLIsReady);
masterLoader.loadURL(firstURLToLoad);
}

private function XMLIsReady (e:Event):void {
photos = new XML(e.target.data); // Load XML Document
loadThumbnails(); // Load photos described in XML Document
}


private function loadThumbnails ():void {
for(var i:int = 0; i < photos.photo.length(); i++) { // Iterate through photos
images[i] = new Thumbnail(i, photos.photo.index[i], xPos, yPos); //create image object
addChild(images[i].getThumbnail()); //Display thumbnails
getXY(); // Get/set X and Y for next thumbnail
}
}

private function getXY ():void {
if (whichColumn != numberOfColumns) { // Not on the last column
whichColumn++; // so go to the next one
}
else { // We are on the last column
whichColumn = 0; // so go back to the first one
whichRow++; // and go to the next row
}
xPos = leftMargin + (whichColumn * xSpace); // New x
yPos = topMargin + (whichRow * ySpace); // and y
}
}


}
}

AdmiralAkbar
03-22-2011, 03:53 AM
Wow this is extremely helpful.

I'm not sure if I'm using the timeline and/or frames. The .fla file is empty except for this as code:

import com.nwstudios.photo.*

Although, I should probably just research the subject since I don't really know anything about how it relates to things written in pure actionscript.

tadster
03-22-2011, 04:13 AM
You don't need to put any code in the fla, in a total oop strategy there would be no code in the fla at all. That one import is why a framescript gets created, so you should be able to take out that code from the fla and then just extend Sprite.

I'm also assuming that you have put PhotoManager as the Document Class, right?


Also, down the road, the MasterLoader class will get more involved, for instance including ways to load multiple images/urls in succession would be handled by the MasterLoader class. What I've showed you is the beginning, like you said, the project will grow to include more functionality. So, you may even consider creating a ThumbnailsMaganer class.

But, my rule of thumb is if a class can be less than 200 lines of code on it's own, there is no real reason to break it up like we are doing. But, breaking it up into individual classes that have single jobs is part of what oop is all about.

AdmiralAkbar
03-22-2011, 04:23 AM
Ahh I see. Yes PhotoManager is the document class. I'm going through a rather long Actionscript 3 book and I think what you're talking about is near the beginning so I'll go review.

I'm trying to expand on the MasterLoader class that you gave me. Running into problems as usual but having fun:)

tadster
03-22-2011, 04:53 AM
ok, here is a few more steps into the MasterLoader that would be possible

package com.nwstudios.photo
{
import flash.events.Event;
import flash.events.EventDispatcher;
import flash.events.IOErrorEvent;
import flash.events.ProgressEvent;
import flash.net.URLLoader;
import flash.net.URLRequest;


public class MasterLoader extends EventDispatcher
{
private var loader:URLLoader;
private var multiURLs:Array = [];
public var percentOfCurrentLoad:int = 0;

public function MasterLoader() {
loader = new URLLoader();
loader.addEventListener(ProgressEvent.PROGRESS, progressHandler);
loader.addEventListener(IOErrorEvent.IO_ERROR, ioErrorHandler);
}

public function loadURL(url:String):void {
loader.addEventListener(Event.COMPLETE, LoadIsComplete);
loader.load(new URLRequest(url));
}

public function loadMultipleURLs(urls:Array):void {
multiURLs = urls;
loadURL(multiURLs.shift());
}

private function progressHandler(e:ProgressEvent):void {
percentOfCurrentLoad = int((e.bytesLoaded/e.bytesTotal)*100);
}

private function LoadIsComplete(e:Event):void {
loader.removeEventListener(Event.COMPLETE, LoadIsComplete);
this.dispatchEvent(e);
if (multiURLs.length != 0) {
loadURL(multiURLs.shift());
}
}
private function ioErrorHandler(e:IOErrorEvent):void {
trace(e.text);
}
}
}


I did not test it, so it could have errors, but it should work.
You can start to see how the class sort of just let's information go out to any other class that may need it. One by simply dispatching the complete event off of itself, and also by having a public property that holds the progress percentage.

In PhotoManager to use the percent you could assign a text field that will display the percentOfCurrentLoad property from the MasterLoader.


Another option would be to just also dispatch the ProgressEvent from the MasterLoader.

Edit:
I hope you can take off from there, I believe that you can, but don't go overboard, and in fact we've already hit a little snag with the percentOfCurrentLoad property, you see, it makes us have to use a loop or re-dispatch the progress event to keep track of when the property changes and/or the progress. If we left the classes together we could just update a text fields text during the initial progress event. So it's a little bit of give and take sometimes, and depends on the overall complexity and scope of your project.

AdmiralAkbar
03-22-2011, 06:14 AM
The code didn't work the way I pasted it (It's possible that I did something wrong), but I'm finally starting to understand event handling in actionscript so I'm just toying with it at the moment.

Still trying to figure it out, but here's where I'm at:

within XMLIsReady function:

trace(e.target.data); // Gives me an error

trace(e.target); // Displays: [object MasterLoader]

within LoadIsComplete function:

trace(this); // Displays: [object MasterLoader]

trace(this.loader); // Displays: [object URLLoader]

trace(this.loader.data); // Displays: Contents of XML File
trace(this.data); // Error

Tracing [object MasterLoader].data, either as e.target.data in XMLIsReady() or this.data in LoadIsComplete, gives me an error.

Tracing [object URLLoader].data as this.loader.data in LoadIsComplete() gives me the content that I am trying to load. I don't know how to access the URLLoader object (or its property 'data') from the XMLIsReady function, but I think that's where my problem is.

The book I'm reading (Essential Actionscript 3.0 O'Reilly) has this to say:

"when ActionScript dispatches an event targeted at an object that is not part of a display hierarchy, that target is the sole object notified of the event."

and

"By contrast, when ActionScript dispatches an event targeted at an object that is part of a display hierarchy, that target and all of its display hierarchy ancestors are notified that the event occurred."

So it seems that, if URLLoader were part of a display hierarchy, it's ancestors (including MasterLoader) would be notified of the event and therefor my content would be accessible through e.target.data (XMLIsReady()) or this.data (LoadIsComplete()).

Or something like that? If none of that makes any sense then feel free to disregard it. My brain hurts I think I need a sandwich.

tadster
03-22-2011, 06:56 PM
Oh! I'm sorry, I overlooked the event situation! What can help is to make a custom event, not only will we be able to use it to get the xml data, but we can use the same event for progress too, so let's call it a MasterLoaderEvent

package com.nwstudios.photo
{

import flash.events.Event;

public class MasterLoaderEvent extends Event
{

public static const MASTER_LOADER_EVENT:String = "masterLoaderEvent";
public var data:Object;
public var progress:int;
public var typeToBe:String;

public function MasterLoaderEvent(data:Object = null, progress:int = 0, typeToBe:String = "masterLoaderEvent"):void {
super(typeToBe);
this.data = data;
this.progress = progress;
this.typeToBe = typeToBe;
}

public override function clone():Event {
return new MasterLoaderEvent(data, progress, typeToBe);
}

}
}



package com.nwstudios.photo
{
import flash.events.Event;
import flash.events.EventDispatcher;
import flash.events.IOErrorEvent;
import flash.events.ProgressEvent;
import flash.net.URLLoader;
import flash.net.URLRequest;


public class MasterLoader extends EventDispatcher
{
private var loader:URLLoader;
private var multiURLs:Array = [];
public var percentOfCurrentLoad:int = 0;

public function MasterLoader() {
loader = new URLLoader();
loader.addEventListener(ProgressEvent.PROGRESS, progressHandler);
loader.addEventListener(IOErrorEvent.IO_ERROR, ioErrorHandler);
}

public function loadURL(url:String):void {
loader.addEventListener(Event.COMPLETE, LoadIsComplete);
loader.load(new URLRequest(url));
}

public function loadMultipleURLs(urls:Array):void {
multiURLs = urls;
loadURL(multiURLs.shift());
}

private function progressHandler(e:ProgressEvent):void {
percentOfCurrentLoad = int((e.bytesLoaded/e.bytesTotal)*100);
this.dispatchEvent(new MasterLoaderEvent(null, percentOfCurrentLoad));
}

private function LoadIsComplete(e:Event):void {
loader.removeEventListener(Event.COMPLETE, LoadIsComplete);
this.dispatchEvent(new MasterLoaderEvent(e.target.data, percentOfCurrentLoad));
if (multiURLs.length != 0) {
loadURL(multiURLs.shift());
}
}
private function ioErrorHandler(e:IOErrorEvent):void {
trace(e.text);
}
}
}


Then in the main class we'll change things up a little bit

//.....
public function PhotoManager(firstURLToLoad:String = "photos.xml"):void {
masterLoader = new MasterLoader();
masterLoader.addEventListener(MasterLoaderEvent.MA STER_LOADER_EVENT, handleAllMasterLoaderEvents);
masterLoader.loadURL(firstURLToLoad);
}
private function handleAllMasterLoaderEvents(e:MasterLoaderEvent):v oid {

if(e.data) {
photos = new XML(e.data); // Load XML Document
loadThumbnails(); // Load photos described in XML Document
} else {
//some text field could get e.progress as the text
}
}
private function XMLIsReady (e:Event):void {
//so now this function is not needed
}


But do you see how complex we are getting? Yes, it all breaks down and will be easier to maintain as the project grows, but if the whole thing could be accomplished in under 200 lines of code, in my opinion, there is no reason to break it into so many parts as we are doing.

AdmiralAkbar
03-22-2011, 10:50 PM
Yes I see what you mean although, as an educational tool this has been a great exercise.

I've created something similar before and, as you probably know, as2 allows you to create functional web sites without actually knowing a whole lot of code. Furthermore, I tend to run into problems once my programs reach a certain size and the code becomes more convoluted.

This has been a great conversation for me and I truly appreciate all of the input and advice.

tadster
03-23-2011, 01:07 AM
Wonderful, I'm glad I could help a bit. That book you have is a very very good book, keep it close by at all times. ;)