Merging data from two different data sets (Facebook & MySQL)

I'm wondering if this is the best way to tackle this issue. I am merging a Facebook users friends data, (from facebook - returns a multi array) with the votes from the users in that list that voted (from MySQL).

This is how I accomplished thi开发者_如何学JAVAs. I'm a junior developer and looking for help on making my code as optimized as possible.

public function getFriendVotes(){
    global $facebook;

    // Get The users friends that use this app from facebook
    $friends = $facebook->api_client->fql_query(
      "SELECT uid, first_name, last_name
        FROM user
        WHERE uid IN (SELECT uid2 FROM friend WHERE uid1=$this->user)"
    );

    // Create an array of just the ids
    foreach($friends as $friend){
      $userids[] = $friend['uid'];
    }

    // Create a string of these ids
    $idstring = implode(",", $userids);

    // Get the votes from only the users in that list that voted
    $result = $this->db->query(
      "SELECT vote, userid FROM user_votes WHERE userid IN ($idstring)"
    );

    // Create a new result set (multi array).  Include the data from the first
    // Facebook query, but include only those who voted and append their votes
    // to the data
    $row = $result->fetch_assoc();
    foreach($friends as $friend){
      if($row['userid'] == $friend['uid']){
        $return[$count] = $friend;
        $return[$count]['vote'] = $row['vote'];
        $row = $result->fetch_assoc();
        $count++;
      }
    }
    return $return;
}

I asume that fql_query does support mysql syntax and it would be more efficient to use LEFT JOIN instead creatig extra query, here is my version of your code:

public function getFriendVotes(){
    global $facebook;

    // Get The users friends that use this app from facebook
    $friends = $facebook->api_client->fql_query("
        SELECT DISTINCT u.uid,u.first_name,u.last_name 
        FROM user AS u 
        LEFT JOIN friend AS f ON uid=uid2 
        WHERE f.uid1='{$this->user}'
    ");
    $arrayUsers = array();
    // Create an array of just the ids
    foreach($friends as $v){
        $arrayUsers[$friend['uid']] = $v;
    }
    unset($friends);

    // Create a string of these ids
    $idstring = implode(",", array_keys($arrayUsers));

    // Get the votes from only the users in that list that voted
    $result = $this->db->query(
      "SELECT vote, userid FROM user_votes WHERE userid IN ({$idstring})"
    );

    $result = array();
    // Create a new result set (multi array).  Include the data from the first
    // Facebook query, but include only those who voted and append their votes
    // to the data
    while($v = $result->fetch_assoc())
    {
        if(isset($arrayUsers[$v['userid']])
        {

            $arrayUsers[$v['userid']] = $v['vote'];

            $result[] = $arrayUsers[$v['userid']];

            unset($arrayUsers[$v['userid']], $v);
        }
    }

    return $return;
}

I can't tell you how your code would perform without measuring and testing. I would look for other issues with your code, that would make it a bit more readable/maintanable. For example:

Create smaller methods.

Inside the main method , I see some chunks of code that are well commented. Why not create a method instead of making a huge comment in the main method?

For example:

// Get The users friends that use this app from facebook
$friends = $facebook->api_client->fql_query(
  "SELECT uid, first_name, last_name
    FROM user
    WHERE uid IN (SELECT uid2 FROM friend WHERE uid1=$this->user"
);
return $friends;

Would make an interesting

functin get_users_friends_from_facebook($facebook){
    // Get The users friends that use this app from facebook
    $friends = $facebook->api_client->fql_query(
      "SELECT uid, first_name, last_name
        FROM user
        WHERE uid IN (SELECT uid2 FROM friend WHERE uid1=$this->user"
    );
    return $friends;
}

In the same manner,

// Get the votes from only the users in that list that voted
$result = $this->db->query(
  "SELECT vote, userid FROM user_votes WHERE userid IN ($idstring)"
);

Is a good candidate to

function get_votes_from_voters(){
    // Get the votes from only the users in that list that voted
    $votes = $this->db->query(
      "SELECT vote, userid FROM user_votes WHERE userid IN ($idstring)"
    );
}

Give variables meaningful names to the context.

$return isn't a good name. Why don't you name it $users_votes for example?

Try to keep the naming convention of your plataform.

Check the apis you're using. Are they using camelCase? Are they using underscores? Try to keep with your libraries and plataform conventions. Check this topic for a good reference.

And welcome to SO. Your code is fine. Try to read some OO principles, you could even cut more lines of your code. All the simple advices I wrote here are avaiable in a great book named Code Complete.

I took points from all your comments and rewrote this method as below. Thanks for all the great input.

public function getAppUserFriends(){
    global $facebook;
    return $facebook->api_client->fql_query(
        "SELECT uid, first_name, last_name
        FROM user
        WHERE uid IN (SELECT uid2 FROM friend WHERE uid1=$this->user)
        AND is_app_user;"
    );
}

public function getFriendVotes(){

    // Get the users friends that use this app
    $friends = $this->getAppUserFriends();

    // Create an array with the ids as the key
    foreach($friends as $v){
        $arrayFriends[$v['uid']] = $v;
    }

    // Create a string of these ids
    $idString = implode(",", array_keys($arrayFriends));

    // Get the votes from only the users in that list that voted
    $result = $this->db->query(
        "SELECT vote, userid
        FROM user_votes
        WHERE pollid=$this->poll
        AND userid IN ($idString)"
    );

    // Pluck out user data from facebook array where the user has voted
    // and add the vote to that array
    while($row = $result->fetch_assoc()){
        $friendsVotes[$row['userid']] = $arrayFriends[$row['userid']];
        $friendsVotes[$row['userid']]['vote'] = $row['vote'];
    }
    return $friendsVotes;
}

Are you having performance troubles in this method? Because unless you are, there's no need to optimize it.

Code first, profile the code, and then optimize where it does the most good.

$friends = $facebook->api_client->fql_query(
  "SELECT uid, first_name, last_name
    FROM user
    WHERE uid IN (SELECT uid2 FROM friend WHERE uid1=$this->user"
);

could probably be shortened to

$userids = $facebook->api_client->fql_query(
  "SELECT uid
    FROM user
    WHERE uid IN (SELECT uid2 FROM friend WHERE uid1=$this->user)"
);

because the uid is the only thing you seem to be using from fb

It was a little hard for me to tell what you are trying to do, but you might consider looking at PHP's array_intersect (and its cousins).

A = {1:'fred', 2:'bob'}
B = {1: 2, 3: 0}

C = array_intersect( array_keys(A), array_keys(B) )
D = {}
foreach (C as c) {
  D[c] = (A[c], B[c])
}

The syntax is off there but I hope it leads you in the right direction.