How is my BlitzMax and OOP programming?
BlitzMax Forums/BlitzMax Programming/How is my BlitzMax and OOP programming?
| ||
I'm looking to get some tips on my BlitzMax and OOP programming. I'm looking for ways I can improve on and some gotchas I don't know about. I am having a bit of a problem with scaling, so I would appreciate help on that. What the program is suppose to do is load six images into a list. And then cycles thru the list and scale each image 10x its size, along with fading it out and then remove the object from the list. Then when the list is empty start over again. Strict Graphics 800,600,0 Global ImageList:TList = New TList 'string array Global arrPath:String[]=["gfx/blue.bmp","gfx/green.bmp","gfx/yellow.bmp","gfx/red.bmp","gfx/orange.bmp","gfx/violet.bmp"] 'loading path to graphics 'and create objects Function LoadPieces() For Local i:String = EachIn arrPath Image.Create(i) Next End Function Type Image Global ImageListCount:Int =0 Field xpos,ypos:Int Field _image:TImage = New TImage Field AlphaLevel:Int =1.0 'instantiate and each object to List Function Create:Image(strPath:String) Local img:Image = New Image img._image = LoadImage(strPath ) ListAddLast ImageList, img End Function 'constructor Method New() ImageListCount:+1 xpos = (GraphicsWidth() / 2) ypos = (GraphicsHeight() / 2) End Method 'go thru each object, scale it and fade object Method Update() Global scale:Int SetBlend ALPHABLEND For Local i:Image = EachIn ImageList MidHandleImage i._image While (1) Cls SetScale scale,scale SetAlpha i.AlphaLevel DrawImage i._image, (i.xpos), i.ypos Flip scale:+1 i.AlphaLevel:-0.5 If scale >= 10 Then ListRemove ImageList, i scale=1.0 i.AlphaLevel=1.0 Exit EndIf Wend Next FlushMem() 'after all objects are destroyed, load objects again and repeat process Reset() End Method Method Reset() If ListIsEmpty(ImageList) = True Then LoadPieces() EndIf End Method End Type Local i:Image = New Image 'main While Not KeyHit(KEY_ESCAPE) i.update() Wend 'destroy i or blitz takes care of this for me when 'the program ends i=Null |
| ||
You've set scale as an Int when it should be a float. Haven't looked at anything else. |
| ||
Well here you go: You've set scale as an Int when it should be a float. Or double. Also, the same is true for Alpha. Field _image:TImage = New TImage As far as I can tell, this is redundant since all images are stored in a List anyway.Additionally you have a problem with creation responsability. Since New() cannot be made private you should use either New(), or the Create Function (Simple Factory). Moving the lines in the New() method to the Create Function would solve this, and additionally you should only increase the count, and add an object to the list, if image loading was actually successful. Additionally there's a potential problem with the Update method. Does having a Global here actually work? Shouldn't scale be a Field (maybe even a Global/static one) like Alphalevel? Also the update method has way too many different responsabilities. It would be better to seperate the update (queue management, etc.) and actual rendering (fade/scale, drawimage) in seperate methods. Additionally Cls/Flip/Flushmem etc. should be moved to the main loop. The main loop could then look something like: While Not KeyHit(KEY_ESCAPE) Cls i.update() i.render() Flip Flushmem WendOther than that it looks perfectly reasonable. |
| ||
OK, I made some changes. How is this?Strict Graphics 800,600,0,20 Type Image Global ImageList:TList Field xpos,ypos:Int Field _image:TImage Field AlphaLevel:Float Field Scale:Float 'instantiate and each object to List Function Create:Image(startXpos:Int,startYpos:Int,imgAlpha:Float,imgScale:Float) Local arrPath:String[]=["gfx/blue.bmp","gfx/green.bmp","gfx/yellow.bmp","gfx/red.bmp","gfx/orange.bmp","gfx/violet.bmp"] For Local i:String = EachIn arrPath Local img:Image = New Image img._image = LoadImage(i) img.xpos = startXpos img.ypos = startYpos img.AlphaLevel = imgAlpha img.Scale = imgScale Next End Function Method New () If ImageList = Null ImageList = New TList EndIf ImageList.AddLast Self End Method 'go thru each object, scale it and fade object Method Update()'img:Image) Scale:+1.0 AlphaLevel:-0.1 End Method Function Render() SetBlend ALPHABLEND If ((ImageList = Null) Or (ImageList.Count() = 0)) Then Return Local i:Image = Image(ImageList.First()) MidHandleImage i._image SetScale i.Scale,i.Scale SetAlpha i.AlphaLevel DrawImage i._image, (i.xpos), i.ypos If i.Scale >= 15.0 Then ListRemove ImageList,i i.Scale=1.0 i.AlphaLevel=1.0 EndIf i.Update() End Function End Type 'main While Not KeyHit(KEY_ESCAPE) Cls If MouseHit(1) Then Image.Create((GraphicsWidth() / 2),(GraphicsHeight() / 2),1.0,1.0) EndIf Image.Render() Flip() FlushMem() Wend |
| ||
Method Update(img:Image) shouldn't have an img parameter. change ImageList to "List" and put it inside Image remove "ImageListCount" Either make Method Render() into a function or take out the loop Looks like youve missed the point of oop, noofense. Prolly see more changes once these are put in. |
| ||
No offense taken. That's why I asked. Now tell me how am I missing the point of OOP. I'm trying to make sure I didn't have the attitude that I understood OOP when I really didn't. |
| ||
Bump. |
| ||
Actually, it looks fine now, its just that it appeared that you were passing img since in a procedural program this is what you would do. Instead, when making methods, you dont pass the object that is being operated on. Also, if you have a function where you pass in a type to modify, particularly if it is a function within a type, it should probably be a method instead. |
| ||
OK, Thanks! |