HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Gathering data from database

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
databasefromdatagathering

Problem

The following code has one of the most confusing lines I've ever wrote. I can imagine ten other way to write it but I do not know which else could be any better.

Note:

Db() is very similar to PDO(), it just extends it adding few features I'm not using here.

Post::addExtra() add abstract datas elaborating database data.

For example he created a $data[13] = $data['from db1'] .' with '. $data['from db2']. These because they are going to be passed to the template.

```
$db = new Db();
$s = new Session();

# Default statement and parameters
$stmt =
"SELECT p.PostPID, p.PostUID, p.PostText, p.PostTime, u.UserUID, u.UserName, u.UserImage, u.UserRep,
(
SELECT COUNT(*)
FROM Flags as f
JOIN Posts as p1
ON p1.PostPID = f.FlagPID
WHERE p1.PostPID = p.PostPID
) as PostFlags
FROM Posts AS p
JOIN Users AS u
ON p.PostUID = u.UserUID
ORDER BY PostTime DESC
LIMIT 0, 30";
$par = array();

# We change the statement if the tab is selected
if ($tab = get('tab')) {
switch ($tab) {
case 'admin':
$stmt =
"SELECT p.PostPID, p.PostUID, p.PostText, p.PostTime, u.UserUID, u.UserName, u.UserImage, u.UserRep,
(
SELECT COUNT(*)
FROM Flags as f
JOIN Posts as p1
ON p1.PostPID = f.FlagPID
WHERE p1.PostPID = p.PostPID
) as PostFlags
FROM Posts AS p
JOIN Users AS u
ON p.PostUID = u.UserUID
WHERE p.PostUID = 1
ORDER BY PostTime DESC
LIMIT 0, 30";
break;
case 'trusted':
if ($s->isLogged()) {
$stmt =
"SELECT p.PostPID, p.PostUID, p.PostText, p.PostTime, u.UserUID, u.UserName, u.UserImage, u.UserRep,
(
SELECT COUNT(*)

Solution

General SQL-related advice: I would factor most of these queries into a single view; you can then add WHERE parameters when selecting from the view. You can also remove the few instances of variable expansion inside the queries, and replace them with named parameters throughout (you have :uid already).

For example:

CREATE OR REPLACE VIEW PostsAnnotated AS
SELECT p.PostPID, p.PostUID, p.PostText, p.PostTime,
    u.UserUID, u.UserName, u.UserImage, u.UserRep,
    (
        SELECT COUNT(*)
            FROM Flags as f 
                JOIN Posts as p1
                ON p1.PostPID = f.FlagPID
            WHERE p1.PostPID = p.PostPID
    ) as PostFlags
    FROM Posts AS p
        JOIN Users AS u
        ON p.PostUID = u.UserUID
    ORDER BY PostTime DESC;

SELECT FROM PostsAnnotated WHERE PostUID = 1 LIMIT 30;
SELECT FROM PostsAnnotated WHERE PostUID IN (
    SELECT TrustedUID
    FROM Trust
    WHERE TrusterUID = :uid)
LIMIT 30;


As far as the code around addExtra: I wouldn't set $posts['Data'] to overwrite it immediately. Instead I would loop on the sql results and append to $posts['Data'] (IIRC the syntax is $posts['Data'][] = $next_elem).

Code Snippets

CREATE OR REPLACE VIEW PostsAnnotated AS
SELECT p.PostPID, p.PostUID, p.PostText, p.PostTime,
    u.UserUID, u.UserName, u.UserImage, u.UserRep,
    (
        SELECT COUNT(*)
            FROM Flags as f 
                JOIN Posts as p1
                ON p1.PostPID = f.FlagPID
            WHERE p1.PostPID = p.PostPID
    ) as PostFlags
    FROM Posts AS p
        JOIN Users AS u
        ON p.PostUID = u.UserUID
    ORDER BY PostTime DESC;

SELECT FROM PostsAnnotated WHERE PostUID = 1 LIMIT 30;
SELECT FROM PostsAnnotated WHERE PostUID IN (
    SELECT TrustedUID
    FROM Trust
    WHERE TrusterUID = :uid)
LIMIT 30;

Context

StackExchange Code Review Q#1426, answer score: 4

Revisions (0)

No revisions yet.