2
\$\begingroup\$

I wrote / modified two scripts to count Facebook and Twitter reactions. Anything I can do to improve these?

Count Facebook Reactions

 class FacebookReactions
 {
    public $url;

    function __construct($url)
    {           
        $query  = 'SELECT like_count, total_count FROM link_stat WHERE url="'.$url.'"';
        $apifql="https://api.facebook.com/method/fql.query?format=json&query=".urlencode($query);

        $json=file_get_contents($apifql);
        $fb_obj=json_decode($json);
        $fb_array=object_2_array($fb_obj);

        $this->total = $fb_array[0]['total_count'];
        $this->like = $fb_array[0]['like_count'];
    }
 }

Count Twitter Reactions

 class TwitterReactions
 {
    public $url = '';
    public $output = array();
    public $count = 0;
    public $total = 100;

    function __construct($url,$total)
    {
        $this->url = $url;

        $query = 'http://search.twitter.com/search.json?q='.$url.'&result_type=mixed&rpp='.$total;
        $reactions=file_get_contents($query);
        $reactions_array=json_decode($reactions, TRUE);

        foreach($reactions_array as $key => $results)
        {
            if(is_array($results)){
                foreach($results as $key => $result){
                    $this->output[$key]['user'] = $result['from_user'];
                    $this->output[$key]['image'] = $result['profile_image_url'];
                    $this->output[$key]['message'] = ereg_replace("[[:alpha:]]+://[^<>[:space:]]+[[:alnum:]/]","<a href=\"\\0\">\\0</a>", $result['text']);
                    $this->output[$key]['date'] =  date('m.d.y',strtotime($result['created_at']));
                }
            }
        }
        $this->count = count($this->output);
    }
 }
\$\endgroup\$
1
\$\begingroup\$

Both classes have undefined or unused class properties. The first class does not use the $url property and never defines the $total or $like properties. The second class just doesn't use the $total property. If these properties really aren't being used, then you can just remove them from the class. However, if you meant to use them, then you will have to do so using the following syntax: $this->url.

PHP will soon be deprecating default method types. In other words, methods must be explicitly defined as public, private, or protected. Right now they are all public by default, but there's no telling how long that will last. Plan for the future and start defining them now so you won't have to later.

I'm not sure where the object_2_array() function is coming from as this is not a function included in the default PHP library. If you are just trying to get the JSON object into an array, then you can do the same thing you did for your twitter JSON object and set the second parameter of json_decode() to TRUE.

Instead of explicitly defining the first array index, just shift the first element off the initial array. Makes it a little more pleasant to read and removes ambiguity as to where the "0" index came from. If you don't want to modify the existing array, you can just assign the value of the first array element to a different variable. However, as it stands this does not appear to matter as the initial array is not being used anyways. Both methods are shown below.

$results = $fb_array[0];
//OR
$results = array_shift( $fb_array );

$this->total = $results['total_count'];
$this->like = $results['like_count'];

When using foreach loops to iterate over an array, it is not always necessary to define the array key as a parameter, only if you are planning on using that value. So, on your first foreach loop in the twitter constructor you can remove the $key => bit as it is unnecessary. Also, keep in mind, it is usually a bad idea to overwrite a variable and should be avoided if not explicitly meant. Even though it is unused, you are overwriting your first $key variable with the second foreach loop. This is bad practice as it could lead to mistakes later. If you need another $key variable you can call it $key2 and the first $key1, or keep the first as it is and call the second key $index. Or anything else really, just so long as it makes sense.

With all that being said, I don't think it was necessary to create these as classes. These are simple enough to where you could have more easily created these procedurally. Additionally, please make sure code you submit for review is functional. As it stands, many of these errors looks like they could have been made from copy-pasting code together.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.