FreeBASIC's Official Forums
|
| View previous topic :: View next topic |
| Author |
Message |
|
|
 Posted: Feb 27, 2010 1:47 Post subject: FreeBASIC Good Practices Discussion |
 |
|
The mistakes most commonly made are rooted in bad practice, so I thought it'd be fun if we could share tips, dos and do nots of programming. Pointers are probably most vital:
1. Type Safety with Pointers
Avoid using a generic pointer type (any/void pointers) if the code isn't designed specifically for it. Example:
| Code:
|
|
#include "fbgfx.bi"
Sub putImage(img As Any Ptr) '' should be: img as fb.Image ptr
Dim As fb.Image Ptr fbImg = cast(fb.Image Ptr, img) '' not safe
Put (0, 0), fbImg, Pset
End Sub
screenres 640, 480, 32
Dim As fb.Image Ptr myImage = imageCreate(640, 480)
putImage(myImage)
Sleep
imageDestroy(myImage) '' free up memory space
|
img as any ptr in putImage should be img as fb.Image ptr. Now, no type-casting is necessary, and you're ensuring that putImage will accept ONLY fb.Image ptrs. This is one way to enfore type-safety and to prevent those annoying "segmentation fault" crash reports.
Also note that we imageDestroy(myImage). Resource management can be confusing, but just know that any memory you allocate (or "reserve" for use by your program) should be deallocated (or "freed" for use by other programs). Always check the manual when creating resources to see how to properly manage them.
2. Null your pointers:
Even though the Operating System will free memory once your program has terminated, while it is still running resources continue to occupy space. Just as dangerous are resources which are allocated, freed, but not zero'd out (made into a null pointer):
| Code:
|
|
Dim As Integer Ptr integerList = New Integer[1000]
For i As Integer = 0 To 999: integerList[i] = Rnd()*1000: Next
Delete[] integerList
integerList = 0 '' necessary if delete[] doesn't automatically 0 integerList
If( integerList <> 0 ) Then
'' if integerList wasn't set to 0, but the memory was freed, this program ideally crashes immediately, but often doesn't.
For i As Integer = 0 To 999: Print integerList[i]: Next
End If |
Pointers are simply integers which "point" to a location in memory (usually RAM). Allocated pointers never equal 0, so we use 0 to indicate "null", or empty pointers. Forgetting to 0 out pointers can lead to thinking memory has been allocated when it hasn't.
Unfortunately, our ideal case of programs crashing immediately upon faulty access does not always occur. Misuse of pointers can cause unpredictable, difficult to trace bugs in code, when the OS doesn't even know you're accessing data you shouldn't be, giving you no indication of faulty memory access.
Last edited by Pritchard on Feb 28, 2010 15:47; edited 1 time in total |
| |
|
| Back to top |
|
 |
|
|
| Posted: Feb 27, 2010 2:18 Post subject: |
|
|
| O wow, I never thought of 2. But I generally don't reuse pointers like that, anyway.. |
| |
|
| Back to top |
|
|
|
|
| Posted: Feb 27, 2010 3:39 Post subject: |
|
|
| agamemnus wrote: | | O wow, I never thought of 2. But I generally don't reuse pointers like that, anyway.. |
It's not necessarily about "reuse". This kind of pointer usage is very common and inherent to most dynamic structures. Linked lists, for example:
| Code:
|
|
'' simple linked list example
Type NodeFwd As Node '' Just to be safe. I don't have fbc so I can't test if I can point to Node within itself...
Type Node
As Integer dat '' data in node - just to show how this works
As NodeFwd Ptr prv '' previous node in list
As NodeFwd Ptr nxt '' next node in list (kind of like arrays)
End Type
'' start our linked list
Dim As Node Ptr cNode = callocate(sizeof(Node))
'' add a node ahead of us
cNode->nxt = callocate(sizeof(Node))
'' add a node behind us
cNode->prv = callocate(sizeof(Node))
'' our list currently looks like this: a<-->b<-->c
'' we're going to do this: a<-->c, so nothing points to b.
cNode->nxt->prv = cNode->prv '' node ahead (c) now points to back (a)
cNode->prv->nxt = cNode->nxt '' node behind (a) now points to front (c)
'' Now make cNode point to a, and remove our island node (b) from existence
Dim As Node Ptr temp = cNode
cNode = cNode->prv
deallocate(temp) |
A linked list will end when either the prv or nxt = 0. It we erased prv or nxt, but forgot to reassign or nullify them, we'd think they were still a part of the linked list and likely run into serious trouble.
Even a simple dynamic array has to keep these things in mind. Callocate can actually return null if memory space is insufficient! |
| |
|
| Back to top |
|
|
|
|
| Posted: Feb 27, 2010 15:50 Post subject: Re: FreeBASIC Good Practices Discussion |
|
|
| Pritchard wrote: | | Even though the Operating System will free memory once your program has terminated |
That generalization is untrue and leads to faultly logic. A lot of OS's do not free resources after a program has terminated which is why you need to do it explicitly. There is no garuntee what OS you will run under or what the OS does or does not do for you as far as resources are concerned. |
| |
|
| Back to top |
|
|
|
|
| Posted: Feb 27, 2010 17:26 Post subject: |
|
|
When allocating memory for dynamic data structures. They should always be put in a OOP style data type, such as using constructors and destructors. In a type defined data type your constructor will allocate and setup your data structure. The destructure will deallocate and clean up your data structure when your program quits. This keeps everything encapsulated so you do not need to worry about the inner workings while concentrating on the 'bigger picture' of your program.
Below is an example of my bitmap loading routine. It is encapsulated in a defined type called BMPimage.
When using this routine I would DIM a BMPimage object.
Example:
DIM AS BMPimage a
Now 'a' is a defined type of BMPimage. Memory for BMP data has not been allocated yet because we did not load a BMP file. When this happends, the defult constructor() is called and NULLs all pointers and variables inside the BMPimage defined type. This is nessesary so that we know that 'a' has no bitmap image. When we load a BMP while we DIM, it would look like this:
DIM AS BMPimage a = BMPimage("bmpfile.bmp")
The line above is the save as previous example, but it will call the constructor(filename as string) instead of the defult one.
When the program or the sub procedure that the BMPimage was defined in exits. The destructor() is called. This is where you deallocate your memory that your object has used.
| Code:
|
|
'BMPloader.bas by Shawn Grieb (c) 2010
'This loader only works for uncompressed 8 or 24 bit bitmaps with the V3 headerinfo.
Type BMPFileHeader Field=1
Dim As Ushort Magic 'Should be "BM"
Dim As Uinteger BMPsize ' Size of BMP file
Dim As Ushort Res1 ' Reserved
Dim As Ushort Res2
Dim As Uinteger OffsetData ' starting addr. of BMP data
Dim As Uinteger BMPheadersize 'ususally 40 bytes
End Type
'BMPheader size
'12 OS/2 V1 BITMAPCOREHEADER OS/2 and also all Windows versions since Windows 3.0
'64 OS/2 V2 BITMAPCOREHEADER2
'40 Windows V3 BITMAPINFOHEADER all Windows versions since Windows 3.0
'108 Windows V4 BITMAPV4HEADER all Windows versions since Windows 95/NT4
'124 Windows V5 BITMAPV5HEADER Windows 98/2000 and newer
Type BMPInfoHeader Field=1
Dim As Integer ImageWidth
Dim As Integer ImageHeight
Dim As Ushort NumPlains ' must be set to 1
Dim As Ushort ImageBitDepth ' the number of bits per pixel, which is the color depth of the image. Typical values are 1, 4, 8, 16, 24 and 32
Dim As Uinteger Compression ' 0 for none
Dim As Uinteger ImageSize ' the size of the raw bitmap data
Dim As Integer WidthPPM ' pixel per meter
Dim As Integer HightPPM ' pixel per meter
Dim As Uinteger NumOfColors
Dim As Uinteger NumOfImpColors
End Type
Type BMPimage
Declare Constructor ()
Declare Constructor (filename As String )
Declare Destructor ()
Declare Sub InitBMP()
Declare Function LoadBMP(filename As String) As Integer
Declare Function LoadBMPnormals(filename As String) As Integer
Declare Sub FreeBMP()
Declare Function BMPptr() As Any Ptr
Declare Function Xsize() As Integer
Declare Function Ysize() As Integer
Dim As Uinteger ImageShiftWidth
Dim As Uinteger ImageBitMask
Dim As Ubyte Ptr pImageData
Dim As Uinteger Ptr pPalette
Private:
Declare Sub ConvertToNormals()
Dim As Integer ImageWidth
Dim As Integer ImageHeight
Dim As Uinteger BitDepth
End Type
Constructor BMPimage ()
this.InitBMP()
End Constructor
Constructor BMPimage (filename As String)
this.LoadBMP(filename)
End Constructor
Destructor BMPimage ()
this.FreeBMP()
End Destructor
Function BMPimage.BMPptr() As Any Ptr
Return this.pImageData
End Function
Function BMPimage.Xsize() As Integer
Return this.ImageWidth
End Function
Function BMPimage.Ysize() As Integer
Return this.ImageHeight
End Function
Sub BMPimage.InitBMP()
this.ImageWidth = 0
this.ImageShiftWidth = 0
this.ImageBitMask = 0
this.ImageHeight = 0
this.BitDepth = 0
this.pImageData = 0
this.pPalette = 0
End Sub
Sub BMPimage.ConvertToNormals()
Dim As Integer c,r,g,b
Select Case this.BitDepth
Case 8
For i As Integer = 0 To 255
c = this.pPalette[i]
r = (c Shr 16) And &h000000ff
g = (c Shr 8) And &h000000ff
b = c And &h000000ff
r = r - 128
g = g - 128
b = b - 128
c = (r Shl 16) Or (g Shl 8) Or b
this.pPalette[i] = c
Next i
Case 24
For i As Integer = 0 To this.ImageWidth * this.ImageHeight - 1
c = this.pImageData[i]
r = (c Shr 16) And &h000000ff
g = (c Shr 8) And &h000000ff
b = c And &h000000ff
r = r - 128
g = g - 128
b = b - 128
c = (r Shl 16) Or (g Shl 8) Or b
this.pImageData[i] = c
Next i
End Select
End Sub
Function BMPimage.LoadBMP(filename As String) As Integer
Dim As BMPFileHeader fileheader
Dim As BMPInfoHeader infoheader
Dim As Ubyte onebyte,rbyte,gbyte,bbyte
Dim As Integer fileoffset, x, y, padded
Dim As Integer filehandle, loaderror = 0
Dim As Any Ptr bufferoffset
filehandle = Freefile
Open filename For Binary As #filehandle
Get #filehandle,,fileheader ' get fileheader
If fileheader.magic = 19778 Then
fileoffset = fileheader.Offsetdata
Get #filehandle,, infoheader ' get infoheader
this.freeBMP() 'free the last image if there was one
this.ImageWidth = infoheader.ImageWidth
this.ImageHeight = infoheader.ImageHeight
this.ImageShiftWidth = Log(Abs(this.ImageWidth)) / Log(2)
this.ImageBitMask = Not (&hffffffff Shl this.ImageShiftWidth)
this.BitDepth = infoheader.ImageBitDepth
Select Case infoheader.ImageBitDepth
Case 8
this.pImageData = allocate(infoheader.ImageWidth*infoheader.ImageHeight+256*4)
bufferoffset = this.pImageData + infoheader.ImageWidth*infoheader.ImageHeight
this.pPalette = bufferoffset
For i As Integer = 0 To 255
Get #filehandle,,this.pPalette[i]
Next i
Seek #filehandle,fileoffset+1
padded = infoheader.ImageWidth Mod 4
For y = -(infoheader.ImageHeight-1) To 0
For x = 0 To infoheader.ImageWidth-1
Get #filehandle,, onebyte
this.pImageData[x+(-y)*infoheader.ImageWidth] = onebyte
Next x
If padded Then
For x = padded To 3
Get #filehandle,, onebyte
Next x
End If
Next y
Case 24
Seek #filehandle,fileoffset+1
this.pImageData = allocate(infoheader.ImageWidth*infoheader.ImageHeight*4)
padded = infoheader.ImageWidth Mod 4
For y = -(infoheader.ImageHeight-1) To 0
For x = 0 To infoheader.ImageWidth*4-1 Step 4
Get #filehandle,, bbyte
Get #filehandle,, gbyte
Get #filehandle,, rbyte
this.pImageData[(-y)*(infoheader.ImageWidth*4)+x] = bbyte
this.pImageData[(-y)*(infoheader.ImageWidth*4)+x+1] = gbyte
this.pImageData[(-y)*(infoheader.ImageWidth*4)+x+2] = rbyte
this.pImageData[(-y)*(infoheader.ImageWidth*4)+x+3] = 0
Next x
If padded Then
For x = padded To 3
Get #filehandle,, onebyte
Next x
End If
Next y
Case Else
Print "Unsupported bit depth: ";filename
Sleep
loaderror = -1
End Select
Else 'Bitmap load failure
Print "Not a BMP file: ";filename
Sleep
loaderror = -1
End If
Close #filehandle
Return loaderror
End Function
Function BMPimage.LoadBMPnormals(filename As String) As Integer
Dim As Integer loaderror = this.LoadBMP(filename)
If loaderror = 0 Then
this.ConvertToNormals()
End If
Return loaderror
End Function
Sub BMPimage.FreeBMP()
If this.pImageData <> 0 Then ' Check if pImageData was allready allocated
deallocate(this.pImageData)
this.ImageWidth = 0
this.ImageShiftWidth = 0
this.ImageHeight = 0
this.BitDepth = 0
this.pImageData = 0
this.pPalette = 0
End If
End Sub
|
|
| |
|
| Back to top |
|
|
|
|
| Posted: Feb 27, 2010 18:46 Post subject: Re: FreeBASIC Good Practices Discussion |
|
|
| 1000101 wrote: | | Pritchard wrote: | | Even though the Operating System will free memory once your program has terminated |
That generalization is untrue and leads to faultly logic. A lot of OS's do not free resources after a program has terminated which is why you need to do it explicitly. There is no garuntee what OS you will run under or what the OS does or does not do for you as far as resources are concerned. |
Afaik only very old OSes and embedded OSes don't deallocate memory on shutdown.
Very old OSes are, euh, old, and embedded OSes applications typically don't terminate anyway.
But maybe I'm wrong, any good examples?
(btw, I agree though that you shouldn't rely on this though, but my reasons are different. It makes it harder to find true memory leaks because of all of the "noise" of undisposed ptrs) |
| |
|
| Back to top |
|
|
|
|
| Posted: Feb 27, 2010 19:07 Post subject: |
|
|
Allocated application (virtual) ram space are handled by OS (Linux and Windows)
but OS resources like bitmaps, GC's, DC's, Brushes, Pens ...
must be deallocated by the application (same on Linux)
Joshy |
| |
|
| Back to top |
|
|
|
|
| Posted: Feb 27, 2010 23:54 Post subject: |
|
|
Summary of latest posts:
1) Make use of Ctors/Dtors to automatically manage resources, from ShawnLG.
2) Be aware that some resources (such as file hooks) must be 'manually' freed, from D.J. Peters.
3) @ShawnLG - Use private to hide data, otherwise your get() functions just add bloat. You over-encapsulate. You're replacing 1 line of code with 1 line of code. A slower line, at that.
Efficiency(%) = (LOCold/LOCnew) * 100%
| Code:
|
|
Type MyType
As Integer foo '' shared by both - if we included this, it would nearly balance out in large number, but for a small example it skews our results
Declare Function getFoo() As Integer '' LOCnew += 1
Declare Sub setFoo(newFoo As Integer) '' LOCnew += 1
End Type
Function MyType.getFoo() As Integer '' LOCnew += 1
Return foo '' LOCnew += 1
End Function '' LOCnew += 1
Sub MyType.setFoo(newFoo As Integer) '' LOCnew += 1
foo = newFoo '' LOCnew += 1
End fun '' LOCnew += 1
Dim As MyType temp
temp.foo = 10 '' LOCold += 1
Print temp.foo '' LOCold += 1
Sleep
temp.setFoo(10) '' LOCnew += 1
Print temp.getFoo() '' LOCnew += 1
Sleep
'' totals: LOCold = 2, LOCnew = 10
'' LOCold/LOCnew * 100% = 20% efficiency. Productivity has actually been reduced 80%.
|
We can factor in efficiency over time by making LOCold fluctuate and keeping LOCnew static (which would give us maintenence costs), but since a simple BMP class isn't expected to change internally, there are few (or no) maintenence costs. You remain 80% less efficient. |
| |
|
| Back to top |
|
|
|
|
| Posted: Feb 28, 2010 17:21 Post subject: |
|
|
| D.J.Peters wrote: | Allocated application (virtual) ram space are handled by OS (Linux and Windows)
but OS resources like bitmaps, GC's, DC's, Brushes, Pens ...
must be deallocated by the application (same on Linux)
|
Afaik only on w9x (which I consider an old OS), and not on NT. (where resources are per process and not a global pool)
I never heard of permanent resource leaks on *nix either, unless the resources were related to very specific circumstances like interprocess communication.
While I'd be interested to see references, that is obviously not what I was talking about, which was mainly memory. |
| |
|
| Back to top |
|
|
|
|
| Posted: Feb 28, 2010 17:38 Post subject: |
|
|
hello marcov
the most windows resources has internal an reference counter
if you don't use DeleteObject(), ReleaseDC() ...
the reference counter will never be zero and memory leaks are the result.
Joshy |
| |
|
| Back to top |
|
|
|
|
| Posted: Feb 28, 2010 19:47 Post subject: |
|
|
| Whether or not the OS handles releasing resources your program requested is pretty irrelevant. I was just using memory and OS as a simple example. This goes beyond system resources, library resources, etc, etc. The idea is that you are responsible for your resources, nobody else. If you allocate the resource you should free resource. It's got to do with the main thread of "good programming practices" (which are not specific to any language). |
| |
|
| Back to top |
|
|
|
|
| Posted: Feb 28, 2010 20:14 Post subject: |
|
|
| Quote: |
3) @ShawnLG - Use private to hide data, otherwise your get() functions just add bloat.
|
Some data items are deliberately left public because it is much faster to access them directly then through a function. I know this is not good practice but there is a line between program readability and speed.
| Quote: |
You over-encapsulate. You're replacing 1 line of code with 1 line of code. A slower line, at that.
|
I assume you mean somthing like this.
| Code:
|
|
Constructor BMPimage (filename As String)
this.LoadBMP(filename)
End Constructor
|
I do this because sometimes I would need to manually load a bitmap. This happends when I create a pointer of type BMPimage. Such as:
DIM AS BMPimage PTR a
At this point, I can't load a bitmap because 'a' is only a pointer. I would need to allocate memory for it.
a = ALLOCATE(SIZEOF(BMPimage))
Now we can load a bitmap, but the constructors and distructors are not called. So I use LoadBMP() and FreeBMP() to manage it manually. In Bowser's Story the bitmap images are loaded dynamicly using pointers. So I have another class structure on top of that such as the Animation Class which manages the allocation and deallocation of the bitmaps. |
| |
|
| Back to top |
|
|
|
|
| Posted: Feb 28, 2010 21:49 Post subject: |
|
|
| Code:
|
|
Function BMPimage.BMPptr() As Any Ptr
Return this.pImageData
End Function
Function BMPimage.Xsize() As Integer
Return this.ImageWidth
End Function
Function BMPimage.Ysize() As Integer
Return this.ImageHeight
End Function |
I was referring to these lines. |
| |
|
| Back to top |
|
|
|
|
| Posted: Jun 10, 2010 12:52 Post subject: |
|
|
| ShawnLG wrote: |
a = ALLOCATE(SIZEOF(BMPimage))
Now we can load a bitmap, but the constructors and distructors are not called. |
Why ALLOCATE? I think, a = New BMPimage(.......) would do the trick (and also call the C/Dtors).
EDIT: Oh, sorry for necroposting. Didn't see the date... |
| |
|
| Back to top |
|
|
|
|
| Posted: Jun 10, 2010 21:56 Post subject: |
|
|
| I'd think it'd be great if this kind of information gets rolled up into a wiki article. At least then it'll get rolled up into the chm, which is what a lot of n00bs use it seems. |
| |
|
| Back to top |
|
|
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
|
|
FAQ
Search
Memberlist
Usergroups
Register
Profile
Log in
