Is there a better way?

Im wondering if some of you GURU's out there could tell me if there is a better way to do this:

RezItem()
{
list Contents = [];
string ItemName = "";
integer NoItems = llGetInventoryNumber(INVENTORY_ALL);
integer x;

for (x = 0; x < NoItems; x++) // create the list of items to give
{
ItemName = llGetInventoryName(INVENTORY_ALL, x);
if (ItemName != llGetScriptName()) Contents += [ItemName];
}

integer ItemsInBox = llGetInventoryNumber(INVENTORY_OBJECT);
float RandomItem = llFrand(ItemsInBox);
integer Item2Rez = llRound(RandomItem);

llSay(0, (string) Item2Rez);
list bar = llListRandomize(Contents, 1);
llSay(0, llList2String(bar, Item2Rez -(1))); //Change to a llRezObject statement later

}

default
{
state_entry()
{
}

touch_start(integer total_number)
{
RezItem();
}
}


// Dont get me wrong... this does work, just wondering if there is a better/ cleaner/ faster way... and I know someone is going to say "If it aint broke...don't fix it" so to you in advance

Here you go. This method adds some changes:

1. The list of objects isn't built every time the object is clicked, rather, it's only built at startup or reset.
2. Corollary to the above: the script resets when an object is added or removed from inventory.
3. If no objects exist within inventory, the script will not allow touches and will return an error message to the owner.

One other thing: This script is intended to rez an object, not to give it to a user, correct? If so, you want to avoid using INVENTORY_ALL and stick to INVENTORY_OBJECT. Other inventory item types can't be rezzed in world, and will cause the script to return an error if you attempt to do so.

list gObjectList; // list of all objects.
   
   RezItem()
   {
   	list object_to_rez = llListRandomize(gObjectList, 1);
   	llSay(0, llList2String(object_to_rez, 0)); //Change to a llRezObject statement later
   }
   
   // builds the list of all objects, returns TRUE if there are objects in inventory, FALSE if not.
   integer buildList()
   {
   	gObjectList = []; // wipe gObjectList.
   	integer num_objects = llGetInventoryNumber(INVENTORY_OBJECT);
   	
   	integer i;
   	for (i = 0; i < num_objects; i++)
   	{
   		gObjectList += llGetInventoryName(INVENTORY_OBJECT,i);
   	}
   	
   	if (num_objects > 0)
   	{
   		return TRUE;
   	}
   	else
   	{
   		return FALSE;
   	}
   }
   
   default
   {
   	state_entry()
   	{
   		integer objects = buildList();
   		if (objects == TRUE)
   		{
   			state running;
   		}
   		
   		else
   		{
   			llOwnerSay("Please add an object to rez.");
   		}
   	}
   	
   	on_rez(integer start_param)
   	{
   		llResetScript();
   	}
   	
   	changed(integer change)
   	{
   		if (change & CHANGED_INVENTORY)
   		{
   			llResetScript();
   		}
   	}
   }
   
   // The reason we use states here is to prevent touch_start from being triggered unless there are objects to rez.
   // If there aren't, the object will not be able to be touched.
   state running
   {
   	on_rez(integer start_param)
   	{
   		llResetScript();
   	}
   	
   	changed(integer change)
   	{
   		if (change & CHANGED_INVENTORY)
   		{
   			llResetScript();
   		}
   	}
   	
   	touch_start(integer total_number)
   	{
   		RezItem();
   	}
   }

Just a minor forumy node, the PHP tag does a reasonable job of formatting most LSL code.

You seem to have been thinking of two things when writing that code. The easy one is giving out a random object:

integer itemCount = llGetInventoryNumber(INVENTORY_OBJECT);
integer itemNumber = llFloor(llFrand(itemCount));
llRezObject(llGetInventoryName(INVENTORY_OBJECT, itemNumber);

Voila, no list needed because the current script will not be an object (or most of the other INVENTORY_* types, if you later modify the code). If you actually want to give anything from the inventory except the current script, it gets a tad more complicated:

integer itemCount = llGetInventoryNumber(INVENTORY_ALL);
integer itemNumber = llFloor(llFrand(itemCount - 1));
// itemNumber is now between zero and one less than the maximum inventory number

if (llGetInventoryName(INVENTORY_ALL, itemNumber) == llGetScriptName()) {
    // We picked our script by accident, set the index to that guaranteed-unpicked
    // element at the end of the inventory. This keeps the probabilities even without
    // needing to build a list.
    itemNumber = itemCount - 1;
}

llRezObject(llGetInventoryName(INVENTORY_ALL, itemNumber), ...);

HTH, Chris

<edit>Unable to type *or* read, good show me.</edit>

Thank you both very much for your replies... I will be using this in a game I am building and will give you both credit for your excellent help.

-Mitz