Better way to do this

Blitz3D Forums/Blitz3D Beginners Area/Better way to do this

Gauge

(Posted 2005) [#1]

This looks kind of ugly,but to help me code a little better, is there a faster or better way for this little function of mine? Or just perhaps a cleaner look?
In my mud engine, when a character moves to a new room, I create an instance of alsohere. Each area has the subtype room, and each room subtype alsohere, etc. And delete/renumber when they leave the room.
Any advice would be helpful. It works fine, and is fast enough right now. Suggetions?

Function AddAlsohere(p.player)
	ax=0:xa=0
	While axo=0
	If area(p\curr_area)\ro[p\curr_room]\ro2[xa]=Null Then
	area(p\curr_area)\ro[p\curr_room]\ro2[xa]=New alsohere
	area(p\curr_area)\ro[p\curr_room]\ro2[xa]\alsoname$=p\name$
	area(p\curr_area)\ro[p\curr_room]\ro2[xa]\playerid=p\playerid
	p\alsoid=xa
	axo=1
	EndIf
	xa=xa+1
	Wend
	WriteLine p\stream, area(p\curr_area)\ro[p\curr_room]\ro2[p\alsoid]\alsoname$+" added ID: "+p\alsoid
End Function

VP	(Posted 2005) [#2]

Only a couple of things, not speed related.

Comment the use of ax, xa and axo or give them more meaningful names. They are obviously global variables, but there's not comment header in your function to say which globals it relies on.

What happens when this function grows by 1000 lines? OK, not likely, but the point remains ;)

Could you replace the globals with a gamestate type instead? Just a thought. I still use globals, so I wouldn't worry about it too much :^)

octothorpe

(Posted 2005) [#3]

1. instead of using a "finished loop" variable (axo), you can often use exit:

while true
	if blah() then
		do_stuff()
		exit
	endif
wend

2. since each player should only have one alsohere entry, perhaps this function should be responsible for calling the function which cleans up the old alsohere entry. if this is the case, it should be named "set" instead of "add".

3. ax is a typo?

4. suggested names for xa: ro2_index, alsoname_index.

5. my typical complaints with arrays used where linked lists should be:

a. every room in your game needs an array big enough to hold an alsohere object for every player

b. when listing the alsohere entries for a room, you need to check every element in the array because your data can become fragmented

there are many ways to pull off linked lists, from adding Field next.alsohere, prev.alsohere and doing all the management work by hand - to using a container class to manage object Handle()s.