Looking for a better way to do this

Looking for a better way to do this - Printable Version

+- SA-MP Forums Archive (https://sampforum.blast.hk)
+-- Forum: SA-MP Scripting and Plugins (https://sampforum.blast.hk/forumdisplay.php?fid=8)
+--- Forum: Scripting Help (https://sampforum.blast.hk/forumdisplay.php?fid=12)
+--- Thread: Looking for a better way to do this (/showthread.php?tid=616994)

Looking for a better way to do this - TakeiT - 13.09.2016

I know it's not pawn, and I know that will piss some of you off for whatever reason, I figure there's a better chance someone has done exactly what I'm trying to do in this forum....

Anyhow, I have a php script to find when the last login was of players, if it's more than 30 days, it prints out their properties to be removed later on. I don't really know of a more efficient way to do this, and this takes like five minutes to process through everything. The script does work exactly as I want it, besides the time delay.

Basically, it loops through all users and checks if their last login was more than 30 days ago, if it was, it loops through all the icons (1000 total) and lists any property owned by them

PHP код:



<?php

                                                            $query = sprintf("SELECT * FROM `users` WHERE `lastlogin` + INTERVAL 30 DAY < NOW()");

                                                            $result = mysqli_query($con, $query);


                                                            if(mysqli_num_rows($result) != 0) 

                                                            { 

                                                                while($row = mysqli_fetch_array($result)) 

                                                                { 

                                                                    $uid = $row['ID'];

                                                                    $name = $row['name'];

                                                                    $lastlog = $row['lastlogin'];

                                                                    

                                                                    $query57 = sprintf("SELECT * FROM `icons` WHERE `Type` = 10 AND `Owner` = '$name'");

                                                                    $result57 = mysqli_query($con, $query57);

                                                                    while($row57 = mysqli_fetch_array($result57)) 

                                                                    {

                                                                        $iid = $row57['ID'];

                                                                        $address = $row57['Address'];

                                                                        

                                                                        echo("[House ID: ".$iid."] [Address: ".$address."] [Owner: ".$name."] [Last Online: ".$lastlog."]<br>");

                                                                    }

                                                                } 

                                                            }

                                                            

                                                        ?>

Re: Looking for a better way to do this - Misiur - 13.09.2016

First rule of webdev: IO (opening files, streams, and querying databases) is the slowest thing in your scripts (this does not mean that database itself is slow, just that sending a query to it is).
Second rule of webdev: If you see a query within a loop, you're doing it wrong.
Third rule: For the love of God, use prepared statements - not only they shield you from SQLi, they are faster for multiple queries with different parameters.
Fourth: Never use strings for association. Your "owner" field should be replaced with user database id.

Here's your structure, and query you need (I'd link SQLFiddle, but somebody is ddosing it I guess)

Код:

CREATE TABLE users
	(`id` int, `name` varchar(25), `lastLogin` DATE)
;
	
INSERT INTO users
	(`id`, `name`, `lastLogin`)
VALUES
	(1, 'Me', NOW() - INTERVAL 2 DAY),
	(2, 'Him', NOW() - INTERVAL 15 DAY),
	(3, 'Her', NOW() - INTERVAL 31 DAY)
;

CREATE TABLE icons
	(`id` int, `owner` int, `type` int(3), `address` varchar(128))
;
INSERT INTO icons
	(`id`, `owner`, `type`, `address`)
VALUES
	(1, 3, 10, "Here"),
	(2, 2, 10, "Ther"),
	(3, 1, 10, "Thur")
;

Код:

SELECT u.id AS id, u.name AS name, u.lastLogin AS lastLogin, i.id AS house_id, i.type AS type, i.address AS address
FROM users u
LEFT JOIN icons i ON i.owner = u.id
WHERE `lastlogin` + INTERVAL 30 DAY <= NOW()
GROUP BY u.id

Check if id is the same as in previous row in case of multiple "icon" table hits.

Re: Looking for a better way to do this - Vince - 13.09.2016

Quote:

Originally Posted by Misiur

Second rule of webdev: If you see a query within a loop, you're doing it wrong.

If you want pure OO then sometimes you don't have a choice. I've had to do this on many occasions during my studies even though I didn't particularly liked it.

Re: Looking for a better way to do this - Misiur - 13.09.2016

True that, even with fancy ORM's this still happens.

Re: Looking for a better way to do this - TakeiT - 13.09.2016

The Owner field is a string because the script saves the "Owner" parameter as the user's name, not their ID.

Thank you though, it does seem to load faster now.

Re: Looking for a better way to do this - CodeStyle175 - 13.09.2016

start using pdo, its better and faster.

Re: Looking for a better way to do this - SickAttack - 14.09.2016

Don't select all fields, and switch to PDO first.