another note on naming convention it's fairly normal to mark private class variables out in some way (usually an underscore before the name (private var _myVar:String;) and leave public variables / getters/setters without - this will give you more options for your variable names (within methods, for instance) and will help you work out whether a variable is available outside the class at a glance
a On 7 July 2010 14:26, allandt bik-elliott (thefieldcomic.com) < [email protected]> wrote: > after a quick glance i'd recommend a couple of things: > > see if you can break your application into smaller chunks that interact > with each other rather than have a single class - one of the benefits of oop > is being able to visualise the problem as distinct tasks that are handled by > separate objects (classes) > > second is just naming conventions - generally only classes should start > with a capital letter - instances (like CityList and PhotoArray) should be > camel case (cityList and photoArray). My own personal preference is to > prefix with the object type (listCities and arPhotos), much like hungarian > notation but there's discussion about how relevant that is in these days of > proper coding environments with automatic code completion (like FlashDevelop > and FDT). > > It's also considered good form for method names to be verbs (getVariable(), > handleEvent(), completeSetup()) although many people like to make their > event handlers follow the form onEnterFrame() / onMouseEvent() (i use > handleEventname() for mine) > > You should try to give values to your class variables in the constructor / > init method or when they're needed if possible. An instance of > myCloseBtn:Button that is added when the variable is declared and then is > never used is memory / cpu wasted > > i'll be looking through the code a bit closer when i get a chance - i'm > sure there's others here who will comment as well > > best > a > > > On 7 July 2010 07:45, J.C. Berry <[email protected]> wrote: > >> Hello all, >> I wondered if I could bother you to review the code below and make >> comments >> and suggestions. It's my first full attempt (from scratch) to create a >> class. I would appreciate any comments you would like to make. Thanks. Oh, >> and to get an idea of how it looks, the project is located at >> http://www.fjroadtrip.com >> Thanks! >> >> >> package{ >> import fl.controls.TextArea; >> import fl.containers.ScrollPane; >> import fl.controls.Button; >> import flash.display.*; >> import flash.events.Event; >> import flash.events.MouseEvent; >> import flash.net.URLRequest; >> import flash.net.URLLoader; >> import fl.managers.StyleManager; >> >> public class DynaMap extends MovieClip{ >> XML.ignoreWhitespace = false; >> private var myXML:XML; >> private var myXMLLoaderReq:URLRequest; >> private var myXMLLoader:URLLoader; >> private var CityList:XMLList; >> private var CityNum:Number = 0; >> private var PhotoArray:Array; >> private var CurrPhoto:Number = 0; >> private var NumOfCities:Number; >> private var myWin:ScrollPane = new ScrollPane(); >> private var DayPicURLReq:URLRequest; >> private var myWinLoader:Loader = new Loader(); >> private var tbaInstance; >> private var CityTextBox:TextArea = new TextArea(); >> private var myCloseBtn:Button = new Button();//closebtn >> private var myNextBtn:Button = new Button();//nextbtn >> >> public function DynaMap(){ >> myXMLLoaderReq = new URLRequest('cities.xml'); >> myXMLLoader = new URLLoader(myXMLLoaderReq); >> myXMLLoader.addEventListener(Event.COMPLETE, xmlLoaded); >> } >> private function xmlLoaded(e:Event):void{ >> myXML = new XML(myXMLLoader.data); >> CityList = new XMLList(myXML.city); >> NumOfCities = CityList.length(); >> //initStage(cityList,0); >> for(var i=0;i<NumOfCities;i++){ >> CityNum = i; >> var DynaBtn = root['city'+CityNum+'_mc']; >> DynaBtn.gotoAndStop(1); >> >> >> DynaBtn.addEventListener(MouseEvent.MOUSE_OVER,DynaBtnRollOverHandler(CityNum)); >> >> DynaBtn.addEventListener(MouseEvent.MOUSE_OUT,DynaBtnRollOutHandler); >> } >> } >> private function ClearScreen(){ >> for (var k:int =0; k < numChildren; k++){ >> >> if(getChildAt(k).name.indexOf('Popout')>-1)removeChildAt(k); >> } >> } >> private function PopoutImgListener(e:Event):void{ >> e.target.content.x = -93; >> e.target.content.y = -47; >> e.target.content.alpha = 0; >> tbaInstance.addChild(e.target.content); >> } >> private function DynaBtnRollOverHandler(cityNum){ >> return function(){ >> root['city'+cityNum+'_mc'].gotoAndStop(2); >> ClearScreen(); >> var PopoutContentLoader:Loader = new Loader(); >> var PopoutContentListener:Object = {}; >> if(cityNum > 25){ >> tbaInstance = new tba2(); >> tbaInstance.x = root['city'+cityNum+'_mc'].x-105; >> tbaInstance.y = root['city'+cityNum+'_mc'].y-55; >> tbaInstance.name = 'cityPopout'+cityNum+'_mc'; >> addChild(tbaInstance); >> }else{ >> tbaInstance = new tba(); >> tbaInstance.x = root['city'+cityNum+'_mc'].x+99; >> tbaInstance.y = root['city'+cityNum+'_mc'].y-55; >> tbaInstance.name = 'cityPopout'+cityNum+'_mc'; >> addChild(tbaInstance); >> } >> >> >> PopoutContentLoader.contentLoaderInfo.addEventListener(Event.COMPLETE,PopoutImgListener); >> var pictURLReq:URLRequest = new URLRequest('images/' + >> CityList[cityNum].images); >> PopoutContentLoader.load(pictURLReq); >> //XML.ignoreWhitespace = false; >> PhotoArray = CityList[cityNum].gallery.split(','); >> if(PhotoArray.length > 0){ >> >> >> tbaInstance.addEventListener(Event.ENTER_FRAME,EnterFrameListener(cityNum)); >> >> tbaInstance.addEventListener(MouseEvent.MOUSE_UP,ClickPopoutListener); >> >> //tbaInstance.addEventListener(MouseEvent.MOUSE_OUT,ClearScreen); >> } >> with(CityTextBox){ >> x = -11; >> y = -53; >> width = 98; >> height = 105; >> selectable = true; >> alpha = 0; >> wordWrap = true; >> verticalScrollPolicy = 'off'; >> horizontalScrollPolicy = 'off'; >> } >> tbaInstance.addChild(CityTextBox); >> } >> } >> private function DynaBtnRollOutHandler(e:Event):void{ >> e.target.gotoAndStop(1); >> } >> private function EnterFrameListener(cityNum){ >> return function(){ >> if(tbaInstance.currentFrame == 8){ >> tbaInstance.getChildAt(1).alpha = >> tbaInstance.getChildAt(2).alpha = 1; >> tbaInstance.getChildAt(1).htmlText = '<b>' + >> CityList[cityNum].name + '</b><br>' + CityList[cityNum].blurb; >> >> tbaInstance.removeEventListener(Event.ENTER_FRAME,EnterFrameListener); >> } >> } >> } >> private function ClickPopoutListener(e:Event){ >> if(PhotoArray.length > 0 && PhotoArray[0].toString()!=''){ >> removeChild(tbaInstance);//delete popout >> myWin.setSize(425, 435); >> myWin.move(0,0); >> addChild(myWin); >> CurrPhoto = 0; >> if(PhotoArray[CurrPhoto].indexOf('*') > -1){ >> DayPicURLReq = new URLRequest('images/' + >> PhotoArray[CurrPhoto].substring(0,PhotoArray[CurrPhoto].length-1)); >> >> >> myWinLoader.contentLoaderInfo.addEventListener(Event.COMPLETE,GalleryLoaded); >> myWinLoader.load(DayPicURLReq); >> myWin.addChild(myWinLoader); >> }else{ >> DayPicURLReq = new URLRequest('images/' + >> PhotoArray[CurrPhoto].substring(0,PhotoArray[CurrPhoto].length)); >> >> >> myWinLoader.contentLoaderInfo.addEventListener(Event.COMPLETE,GalleryLoaded); >> myWinLoader.load(DayPicURLReq); >> myWin.addChild(myWinLoader); >> } >> } >> } >> private function GalleryLoaded(e:Event){ >> if(CurrPhoto<PhotoArray.length-1){ >> if(PhotoArray[CurrPhoto].indexOf('*') > -1){ >> e.target.content.y = 0; >> e.target.content.x = 68; >> myCloseBtn.setSize(45,22); >> myCloseBtn.label = 'close'; >> myCloseBtn.move(310,0); >> addChild(myCloseBtn); >> >> myCloseBtn.addEventListener(MouseEvent.MOUSE_UP,CloseGalleryWin); >> myNextBtn.label = 'next'; >> myNextBtn.setSize(50,22); >> myNextBtn.move(260,0); >> addChild(myNextBtn); >> >> myNextBtn.addEventListener(MouseEvent.MOUSE_UP,NextPicListener);//next in >> gallery >> }else{ >> e.target.content.y = 60; >> e.target.content.x = 0; >> myCloseBtn.setSize(45,22); >> myCloseBtn.label = 'close'; >> myCloseBtn.move(376,60); >> addChild(myCloseBtn); >> >> myCloseBtn.addEventListener(MouseEvent.MOUSE_UP,CloseGalleryWin); >> myNextBtn.label = 'next'; >> myNextBtn.setSize(50,22); >> myNextBtn.move(325,60); >> addChild(myNextBtn); >> >> myNextBtn.addEventListener(MouseEvent.MOUSE_UP,NextPicListener);//next in >> gallery >> } >> }else{ >> if(PhotoArray[CurrPhoto].indexOf('*') > -1){ >> e.target.content.y = 0; >> e.target.content.x = 68; >> myCloseBtn.setSize(45,22); >> myCloseBtn.label = 'close'; >> myCloseBtn.move(310,0); >> addChild(myCloseBtn); >> >> myCloseBtn.addEventListener(MouseEvent.MOUSE_UP,CloseGalleryWin); >> //removeChild(myNextBtn); >> }else{ >> e.target.content.y = 60; >> e.target.content.x = 0; >> myCloseBtn.setSize(45,22); >> myCloseBtn.label = 'close'; >> myCloseBtn.move(376,60); >> addChild(myCloseBtn); >> >> myCloseBtn.addEventListener(MouseEvent.MOUSE_UP,CloseGalleryWin); >> //removeChild(myNextBtn); >> } >> } >> } >> private function CloseGalleryWin(e:Event){ >> removeChild(myWin); >> removeChild(myCloseBtn); >> removeChild(myNextBtn); >> } >> private function NextPicListener(e:Object):void{ >> removeChild(myCloseBtn); >> removeChild(myNextBtn); >> CurrPhoto++; >> if(CurrPhoto<=PhotoArray.length-1){ >> if(PhotoArray[CurrPhoto].indexOf('*') > -1){ >> DayPicURLReq = new URLRequest('images/' + >> PhotoArray[CurrPhoto].substring(0,PhotoArray[CurrPhoto].length-1)); >> >> >> myWinLoader.contentLoaderInfo.addEventListener(Event.COMPLETE,GalleryLoaded); >> myWinLoader.load(DayPicURLReq); >> }else{ >> DayPicURLReq = new URLRequest('images/' + >> PhotoArray[CurrPhoto].substring(0,PhotoArray[CurrPhoto].length)); >> >> >> myWinLoader.contentLoaderInfo.addEventListener(Event.COMPLETE,GalleryLoaded); >> myWinLoader.load(DayPicURLReq); >> } >> } >> } >> } >> } >> >> -- >> J.C. Berry, M.A. >> UI Developer >> 619.306.1712(m) >> [email protected] >> _______________________________________________ >> Flashcoders mailing list >> [email protected] >> http://chattyfig.figleaf.com/mailman/listinfo/flashcoders >> > > _______________________________________________ Flashcoders mailing list [email protected] http://chattyfig.figleaf.com/mailman/listinfo/flashcoders
