Is this code correct?

This forum is now closed as part of retiring phpBB2.
Forum rules
READ: phpBB.com Board-Wide Rules and Regulations

This forum is now closed due to phpBB2.0 being retired.
Post Reply
User avatar
Vampy
Registered User
Posts: 140
Joined: Mon May 12, 2003 4:25 pm

Is this code correct?

Post by Vampy »

Hi! I was wondering if anyone here could help me look through the following lines of code and tell me whether it is corrrectly written for what I want it to do. Basically, what I need the code to do is to compare $itemfound with the user item. If the user already has the item, the code will get a different $itemfound until the $itemfound is something that the user does not have.

Code: Select all

do {
		// what item did the user won??
			if ($itemtype == 'C') 
			{ 
				$items_array = explode("##", $items_ch);
				$newrand = rand(1,10); 
				$itemfound = $items_array[$newrand];
			} 
			if ($itemtype == 'UC') 
			{ 
				$items_array = explode("##", $items_ch);
				$newrand = rand(1,10); 
				$itemfound = $items_array[$newrand];
			} 
			if ($itemtype == 'R') 
			{ 
				$items_array = explode("##", $items_ch);
				$newrand = rand(1,10); 
				$itemfound = $items_array[$newrand];
			} 
			if ($itemtype == 'VR') 
			{ 
				$items_array = explode("##", $items_ch);
				$newrand = rand(1,10); 
				$itemfound = $items_array[$newrand];
			}
			if (preg_match("/^@([A-Za-z]+[\_*|\s*|\-*|\&*|\(*|\)*]*)+/", $itemfound, $matches)) {
				$temp_avatar_name = preg_replace("\@", "", $matches[0]);
				$find_item_sql = "select * from phpbb_shops where name=".$item_avatar_name;
				if ( !($find_item_result = $db->sql_query($find_item_sql)) )
				{
					message_die(GENERAL_MESSAGE, 'Fatal Error: '.mysql_error());
				}
				$find_item = mysql_fetch_array($find_item_result);
				if (mysql_num_rows($find_item_result) < 1) { message_die(GENERAL_MESSAGE, 'No such item exists!'); }
				$temp_find_item = "ß".$find_item['item_image_dir']."Þ";
				$temp_user_bought_avatar = $row['user_bought_avatar'];
				if (substr_replace($temp_user_bought_avatar, $temp_find_item, strpos($temp_user_bought_avatar, $temp_find_item), strlen($temp_find_item))) {
					$item_repeat = 1;
				}
				else {
					$item_repeat = 0;
				}
			}
			else {
				$item_repeat = 0;
			}
		}
		while ($item_repeat != 0);
Did I write the do and while codes properly and whether does it go into infinte loop? Please tell me if there is a more efficient way to wrote the codes to achieve what I need.

Thanks.
Julian
User avatar
Ptirhiik
Registered User
Posts: 7411
Joined: Mon Jan 06, 2003 10:36 pm
Contact:

Post by Ptirhiik »

If you preg_match is always verified, you never go outside your loop. preg_match_all() won't be the good one to use here to avoid a non-itered loop ?
User avatar
Vampy
Registered User
Posts: 140
Joined: Mon May 12, 2003 4:25 pm

Post by Vampy »

Hi! The preg_match will only be verified when the $itemfound has a particular value. If that is true then it will go and match with the user items. If that is also a match, then $item_repeat will be set to 1 and the loop will execute the previous codes again to obtain a new $itemfound value. If however, preg_match is not successful, then it will set the $item_repest to 0 and will exit the loop. The same goes if although preg_match is valid but if the user has no such item then, it will also exit the loop.

Are you saying that the codes I've written will not work as I want it to (i.e. no matter what happens the loop will go into infinity)? You mentioned that preg_match_all? Ididn't use that in my codes. Are you saying to use it instead of just preg_match?

Thanks for your kind input.
Julian
User avatar
Ptirhiik
Registered User
Posts: 7411
Joined: Mon Jan 06, 2003 10:36 pm
Contact:

Post by Ptirhiik »

Yep, I think the problem stands there. Actually, it is more a way to work : you should never use a loop for which you don't know how many iterations will be necessary to go outside. The only exception is a sequential read of a list (meaning no jump in from one item to another, breaking the flow).

A safer approach will be so :
- determine the number of iterations,
- loop (with ie for rather than while)

Doing so your exit condition is know before enter the loop, and it will makes your algo clearer (so less chances to have an error in it).
User avatar
Vampy
Registered User
Posts: 140
Joined: Mon May 12, 2003 4:25 pm

Post by Vampy »

Hi! I know what you mean. But the problem lies with the fact that I indeed have no idea how many iterations it would take to get a $itemfound for which the user does not have in his account.

The whole point is that I do not want to give $itemfound to the user for which the user already have. No point in him/her having the same item in his inventory. As I understand with the "for" loop you will need to kow the eact number of iterations but in this case, I will never know the exact iterations needed.

Thank you for your input. I can't see another way to achieve what I need and would appreciate if you could enlighten me. I wonder whether "case:" can help me with achieving what I need. What do you think?
Julian
User avatar
Ptirhiik
Registered User
Posts: 7411
Joined: Mon Jan 06, 2003 10:36 pm
Contact:

Post by Ptirhiik »

You know the number of items the user have. If I understood your purpose, you want do add a new if it doesn't stand in the existing ones. Getting first the list of items belonging to this user will gives you the number of items to check with your new item. Doing so you will :
- remove your loop ( only looping on the rand() ),
- reduce greatly the number of needed sql requests.
User avatar
Vampy
Registered User
Posts: 140
Joined: Mon May 12, 2003 4:25 pm

Post by Vampy »

I hail to the almighty. :mrgreen: Thank you so much. I think I now understand what you are saying.
Julian
Post Reply

Return to “[2.0.x] MOD Writers Discussion”