How to make this OOP?

Hello,
This section of code opens a text file and examines it, byte by byte, for preconceived values.
It works. I have no issues with how it works. There are more checks to be done on the rest of the contents of the file but they don't matter for todays question.

This is all Procedural Programming (from what i understand).
Can you give me some tips on how i should change it to be more OOP?

Files will always be of varying length/size and only the first part will be static (matching byte by byte).

I am happy with responses saying "hey it works, you don't need to change it" but since i am still learning i would like to know the different styles.

Regards
Glenn

SuperStrict

Global FinalOutput:String[100]	' 100 element string array. This is used to store the output comments
Global sFinalOutput:String		' Temporary string to write messages to the array
Global ArrayCounter:Int = 0		' Keep track of where we are up to in the array

Local FileName:String = RequestFile( "Select FTI Client file to be tested","All Files:*" )

Global FileInput:TStream = OpenStream(FileName)
Global LTName:String
Global NumberOfNULLs:Int
Global BadFile:Int = False

If(FileType(FileName)) = 1 		' Only files can be chosen, or the Cancel button is pressed
	' Open the file and read in the first line.
	Local FirstLine:String = ReadLine(FileInput)
	' Check that the first line includes one of the pre-defined values
	Local FirstLineType:String = CheckFirstLine(FirstLine)

	Select FirstLineType
		Case "ChangeLog"		' This is a Change Log message
			FinalOutput[ArrayCounter] = "This appears to be a Change Log file."
			LTName = "CP_CHNG_LOG_2"
			NumberOfNULLs = 83
		Case "Manifest"			' This is a Manifest message 
			FinalOutput[ArrayCounter] = "This appears to be a Manifest file."
			LTName = "CP_FCPS_BOOK"
			NumberOfNULLs = 84
		Case "TrackTrace"		' This is a Track and Trace message
			FinalOutput[ArrayCounter] = "This appears to be a Track and Trace file."
			LTName = "CP_SENQ"
			NumberOfNULLs = 89
		Case "FreightForward"	' This is a Freight Forward message
			FinalOutput[ArrayCounter] = "This appears to be a Freight Forward file."
			LTName = "FF_JOBS"
			NumberOfNULLs = 89
		Default
			FinalOutput[ArrayCounter] = "This doesn't appear to be a valid FTI Client message structure. Please contact ECL customer Integration Team."
			Notify ("This doesn't appear to be a valid FTI Client message structure.~nPlease contact ECL Customer Integration Team.")
			BadFile = True
	End Select
	ArrayCounter:+1

	' Close the file and REOPEN it so we can check if the FTI Header has been written correctly
	CloseStream(FileInput)
	
	If BadFile = False Then
		' Check the FTI Header portion of the file.
		FileInput:TStream = OpenStream(FileName)
		Local CheckFTIHeaderSuccess:Int = CheckFTIHeader(FirstLineType, LTName, NumberOfNULLs)
		CloseStream(FileInput)
		' 
	EndIf

	' Write the Output Log File
	Local success:Int=CreateFile(CurrentDir()+"\FTI_Output.txt")	' this will delete an existing file and create a new file
	If Not success RuntimeError "Error Creating Output File"

	Local FileOutput:TStream = WriteFile(CurrentDir()+"\FTI_Output.txt")
	
	For Local sOutput:String = EachIn FinalOutput
		WriteLine FileOutput, (sOutput)
	Next
	
	CloseFile(FileOutput)
Else	
	Notify ("The Cancel button was chosen in the File Selector")
EndIf

End

Function CheckFTIHeader:Int(MessageType:String, LTName:String, NumberOfNULLs:Int)
	Local Result:Int = True
	Local i:Int = 1
	Local bByte:Byte = ReadByte(FileInput)
	Local sByte:String = ""
	
	' Check the first byte
	If bByte <> 152 Then 
		FinalOutput[ArrayCounter] = "The FTI Client Header should have a (chr(152)) as the first byte but it doesn't - FAILURE."
		Result = False
	Else
		FinalOutput[ArrayCounter] = "The FTI Client Header should have a (chr(152)) as the first byte and it does - SUCCESS."
	EndIf
	ArrayCounter:+1
	
	' Check the second byte
	bByte:Byte = ReadByte(FileInput)
	If bByte <> 1 Then 
		FinalOutput[ArrayCounter] = "The FTI Client Header should have a chr(1) as the second byte but it doesn't - FAILURE."
		Result = False
	Else
		FinalOutput[ArrayCounter] = "The FTI Client Header should have a chr(1) as the second byte and it does - SUCCESS."
	EndIf
	ArrayCounter:+1
	
		' Check the correct LTName is in the header
		For i = 1 To Len(LTName)
			sByte:+Chr(ReadByte(FileInput))
		Next
		If sByte <> LTName Then
			FinalOutput[ArrayCounter] = "The FTI Client Header should have " + LTName + " as the 3rd to " + (Len(LTName)+3) + "th bytes but it doesn't - FAILURE."
			Result = False
		Else
			FinalOutput[ArrayCounter] = "The FTI Client Header should have " + LTName + " as the 3rd to " + (Len(LTName)+3) + "th bytes and it does - SUCCESS."
		EndIf
		ArrayCounter:+1
		' Check the number of NULLs
		Local NullResult:Int = True
		For i = 1 To NumberOfNULLs
			bByte = ReadByte(FileInput)
			If bByte <> 0 Then
				NullResult = False
			EndIf
		Next
		If NullResult = False Then
			FinalOutput[ArrayCounter] = "The FTI Client Header should have " + NumberOfNULLs + " NULLs (Chr(0)) but it doesn't - FAILURE."
			Result = False
		Else
			FinalOutput[ArrayCounter] = "The FTI Client Header should have " + NumberOfNULLs + " NULLs and it does - SUCCESS."
		EndIf
		ArrayCounter:+1
		
		' Check that there is no CRLF and that it flows directly into the Message Header
		bByte = ReadByte(FileInput)
		If bByte = Asc(Left(LTName,1)) Then 
			FinalOutput[ArrayCounter] = "The FTI Client Header should be DIRECTLY followed by first part of the " + MessageType + " Message Header - SUCCESS."
		Else
			FinalOutput[ArrayCounter] = "The FTI Client Header should be DIRECTLY followed by first part of the " + MessageType + " Message Header but it doesn't - FAILURE."
			Result = False
		EndIf
		ArrayCounter:+1

	Return Result
End Function


Function CheckFirstLine:String(Line:String)
	' check the first line for the message type
	Local Result:String

	If Line.contains("CP_CHNG_LOG_2") Then Result = "ChangeLog"
	If Line.contains("CP_FCPS_BOOK") Then Result = "Manifest"
	If Line.contains("CP_SENQ") Then Result = "TrackTrace"
	If Line.contains("FF_JOBS") Then Result = "FreightForward"
	
	Return Result
End Function

Going OOP not always makes things easier and it also does not always make as much sense as the OOP fans claim.

-- The -- advantage of OOP, in my (sometimes not so) humble opinion, is encapsulation.

In your case, this means that you basically put the whole thing in an own class and instead of declaring all variables as globals, you make them private. You can pass the name of the file you want to examine to the constructor of the class and let the method that does the actual work return the result array. This way, all the temporary and worker variables are hidden from the user and the rest of the code --- encapsulation.

yes yes, "\o/ yay \o/" for encapsulation and datahiding. Globals might look tempting at first sight, but are immensely evil in the long rung when an app keeps expanding and expanding and you run out of sense making namespace.

While we're at it, Wil Shipley (Delicious Monsters) recently wrote some interesting things about OOP in his blog:

http://wilshipley.com/blog/2007/05/pimp-my-code-part-14-be-inflexible.html

As the name suggests Object-Oriented programming means you should be thinking about objects. I've taken a quick peek at your code, and I don't see anything crying out to be an object. That's the test I always make. Is there an obvious object? In your code, I don't see it, but then you know your code better than I do.

If you had it more generic, say a function ( or set of functions ) to read and write to .ini files then you could have any IniFile object, read them, write them, add entries, remove entries. That's your object. Indeed entries could be objects too. In your code, you look to be hard coding for a specific purpose and as such, I don't see much reusable code that I would call related to an object.

Just my 2c.

Gabriel - nothing sort of jumped out at me either but it is always good to ask other people.
CS_TBL and Winni - I will need to read up a bit more on your comments.

All feedback appreciated.

Cheers
Glenn

I agree in part with Gab, cs and wini, but to continue their comments, if all the functions are being passed the same first parameter ie

RotateCamera(Camera,rot):PositionCamera(Camera,x,y,z)

etc then its probably a case for an object, in the pure sence.

However, in the drive to never have a Global, I would also say that if you do have seven globals that are always to be used together, I would be very very tempted to put them into a Type, so as to have them in the same namespace, probably keeping them as Globals.

Once I'd done this, it then wouldnt be behond the relems of possibility, that I would also put the fuctions that used those globals inside that "Namespace" (ie Object)

It wouldnt be an Object in the This is an "Object" sence, ... but well you know what I mean

H&K - Not sure if i understand correctly.
I create an object and have those values still as Globals? (or locals?) and put the functions into the type too? Do these functions then become methods?

I will have a stab at OOPing this on the weekend based on suggestions from you all.

Cheers
Glenn

1) As Globals, and they stay as static to that Type, that mean that even if you made 20 instances of that type, there would only be one of each global
2) No, they stay as functions of the type

What you have is Variables and Procedures of either the type/class itself or instances/objects of that type

Globals and Functions defined within the type are part of the type, but the dont belong to any instance of it, really you can look at them as simply haveing a longer name than before

AType.GlobalVariable  AType.Function:Int() etc

Fields and Methods on the other hand belong to a specific instance of that Type, specificly when in Methods the type "Self"

As an example, if I want to create a new Instance of MyType, I could do this

MyInstance:MyType = New MyType
MyInstance.MethodAllocate(Parameters)

or

MyInstance:MyType = New MyType
MyType.FunctionAllocate(MyInstance,Parameters)

The first one calls the allocate Method inside MyType. This method knows that is was called by MyInstance, and infact thinks of itself as part of MyInstance and reffers to it as "Self"

The second example the allocate is a member of MyType, but you can see that it isnt called by the instance, but rarther by the Type itself, (Self still works, but Mark has said that it really makes no sence in a function in a type, and is really only there for lazy typers with intellisence in the IDE), and MyInstance needs to be passed as a parameter.

The way I do this allocation (for example) is

MyInstance:MyType = MyType.FunctionCreate(Parameters)

The function then News the Instance and passes the Parameters to the Allocate Method.
(I do this for a few reasons, partly becasue it make inheritance easier, but mostly because I dont like NEW as a command in "open" code)

I do agree with Gab, for whilst I would put it all inside a type, that would only be because in my mind it is tidyer, not because Im makeing in any real sence an "Object" in the OOP meaning. All I'd really be doing is creating a subProgram/namespace that contained all the variables and fucntions, then I would just call it as though each function was still a normal function, but now with a longer name.

Another bit o' info about avoiding globals. Imagine you're creating a pinball game using globals. You have global variables like BallX, BallY, BallSpeed, BallAngle, BallRadius, BallImage, LevelWidth, LevelHeight, LevelMap, Score, Highscore, Lives, etc. etc.

All nice, the game works. And then suddenly you want to make a 2-player option where you'd have 2 of such pinball machines next to eachother. It would mean you need all those global variables twice. What could you do? Make 'em an array of 2? Could be, but those variables are everywhere, it's quite tiresome to run through the whole code to fix things.

OO?

You'd have all those varibles in a pinballmachine type, and then simply:

Player1machine:TPinball=New TPinball
Player2machine:TPinball=New TPinball

or

Local Player:TPinball[2]
Player[0]=New TPinball
Player[1]=New TPinball

That's far more easier than to manually change all those variables.. and this is actually a very simple example.
Ofcourse, do you need Bmax for this? Of course not, I was doing this in B+ already with banks, but instead of "Player1.shootball" I'd be doing "shootball Player1".

The problem with globals is the sheer lack of namespace you'll run into, sooner than you'd hope. If you have a lot of different 'objects' and they're all destined to use globals, then you either get cross references, or you're really cooking some extravagant names.

Another nice one, imagine you'd be making your own gadgets (knobs, sliders, buttons, etc.). If you would be doing this without OO and with global functions, then you'd get stuff like: UpdateKnob MyKnob, UpdateSlider MySlider, UpdateButton MyButton. While not the end of the world, it's still some typing work.

When doing OO, an update method is part of such a gadget. So it'll be MyKnob.update, MySlider.update, MyButton.update. What you gain is that you loose those global functions. Within the type (e.g. in some method of 'Slider') you only need to type 'update', that's what you do most of the time anyway: updating from within the object.

About namespace, when still doing the non-OO way and use global functions, it could very well be that you could commit yourself to names you're aware of so that you will avoid nameclashes yourself. If you keep some guidelines this could very well possible, since *you* are the one who's managing the whole shebang.
And now you want to use library x from coder y (since it's such a good library from a respective coder) .. now all of a sudden you don't control your own sources anymore since it has external code, which is usually quite hard to read. (at least it is for me!) If it's such a respective coder then you're lucky since all the stuff would prolly be tucked away in types, just as I wrote above. But what if the library is good, but not very cleverly written? What if you were using some UpdateKnob function, but the coder of that lib does so too (for a whole different reason/situation of course)? Here you see the benefits of datahiding. You can hide zillions of variables and methods into a class. Without OO you'd have zillions of variables and functions all being global and all being around messing up with whatever comes next -which you can't always know!-.

So, what globals are allowed? Well, depending on the scale of your app there are perhaps some. Perhaps -for a game- having variables such as gamewidth and gameheight as a global seem obvious, but is it? In most cases it'll do.. but what if you want to run multiple instances of the game in multiple windows, each with its own size? True, perhaps not a realistic example, but technically it's a perfect example. If you store gamewidth and gameheight in your game type then you can do this. When you make 'em globals, then you can't.. (unless you make 'em an array orso)

H&K and CS_TBL - Thank you for your responses.
This is why i posted the original question...

1 - So i can get some specific feedback to my code
2 - So very well written explanations in a more generic sense will be posted for other readers.

Thank you again.

Glenn