Basic Server Framework

[TheMinistered]

I figure some of you guys out there would like to disect my simple server framework, which is going to be further designed into a small-userload game server.

Due to lack of good hosting, I decided to rely on planet-source-code for hosting:
http://www.pscode.com/vb/scripts/ShowCode.asp?txtCodeId=56388&lngWId=1

[TheMinistered]

no thoughts, remarks, or comments?? hmm?

[Banana fanna fo fanna]

Writing a server in VB is a *bad thing*.

[TheMinistered]

* TheMinistered Puts extended emphasis on small-userload

[Banana fanna fo fanna]

* Banana fanna fo fanna puts extended emphasis on crappy program structure options

[Grok]

Nice and cleanly formatted.  Makes it easy to view.

Drop all the hungarian notation.  It is not necessary, does not add clarity to most VB programs.  m_objClients.Item()... really more clear than Clients.Item()? No.

Complete lack of comments will haunt you eventually, if not on this tiny server then someday elsewhere.  Add simple one-liners better than nothing ... "Parses incoming data for client.."

Upgrade your error handlers.  You should always resume, not just rely on VB falling out of a procedure to clear the error.  Better to purposefully clear it by resuming to an exit point, if you are otherwise just falling out the procedure bottom.

In your upgraded error handling, add logging.  All error traps should write to the error log file, at least the module name, procedure name, error number and description.  If a critical part of the code with many possible errors, add line numbers to the VB6 code and put the line number in the error report.

Check the socket state before doing operations like .Close that would generate an error.  (Form_Unload)

I'm a little confused why objDatabase is Public in the global module, but then instantiated in the form.  If there's only one instance of it, and that instance always exists, just instantiate it in Main(), prior to the form loading?

Speaking of Main(), set your startup object to Sub Main() and load your form from there.  Makes it much easier to control program startup if you don't have to fiddle with a form while loading settings.

More on error handlers, functions like Add(), for your classes, should definitely have error handling and logging.

You forgot to set your Item() properties as the default property for your classes.

Also, your NewEnum() do not have the required -4 procedure ID and hidden, so will not support For Each, as written.  (clsClients is only one I checked this on)

In clsDatabase, I urge you to set your connection string from a registry entry, or at least an INI file.  You can still have the default connection string in your source, but a modifiable version should be in a user-maintainable location.  Besides, as written, you are limiting your users to MySQL.  If I can change the Connection String, I could use MSSQL, Access, Oracle, etc.

When you call SendPacket, you're sending whatever is in the buffer and then clearing the buffer?  OK, just checking if that was your intent.

Good start.

[TheMinistered]

I appreciate all the comments Grok, I'll definantly put some time into error handling because it saves time when it comes to those stupid bugs that are hard to find.  As for the SendPacket comment, I believe although am not 100% sure that when you use SendData on a winsock control it will make a copy of the buffer thus the original isn't needed.  However, if you are suggesting that it doesn't I'll simply clear the last packet sent via the sendcomplete event.