PHP create JSON file from Twitter API

Question

I've been reading more on PHP JSON and APIs I wanted to play around and pull a user's tweets so I wrote a function that requires the TwitterAPIExchange wrapper.

The code:

ini_set('display_errors', 1);
require_once('TwitterAPIExchange.php');
get_twitter_json();


function get_twitter_json() {
    $username       = 'foobar';
    $count          =   5;
    $json_folder    = 'json';
    $pull_time      =   1;

    $settings = array(
        'oauth_access_token'        => "",
        'oauth_access_token_secret' => "",
        'consumer_key'              => "",
        'consumer_secret'           => ""
    );

    if (is_dir($json_folder)) :
        echo "Directory $json_folder exists<br />";
    else :
        echo "Did NOT find directory $json_folder<br />";
        exit();
    endif;

    $file_loc   = "json/twitter.json";
    $cachetime  = @filemtime($file_loc);
    $diff       = time() - $cachetime;

    if ($diff <= 60 * $pull_time) :
        $results    = json_decode(file_get_contents($file_loc));
    else :
        $url = 'https://api.twitter.com/1.1/statuses/user_timeline.json';
        $getfield = '?screen_name=' . $username . '&count=' . $count;
        $requestMethod = 'GET';
        $twitter = new TwitterAPIExchange($settings);
        $results = json_decode(
            $twitter->setGetfield($getfield) 
                    ->buildOauth($url, $requestMethod) 
                    ->performRequest(), true
        );

        $i = 0;
        if (isset( $results['error'])) :
            echo 'Issue found in request';
            exit();
        else :      
            foreach($results as $tweet) : 
                $tweets[$i]['created']      = $tweet['created_at'];
                $tweets[$i]['text']         = $tweet['text'];
                $tweets[$i]['username']     = $tweet['user']['name'];
                $tweets[$i]['screenname']   = $tweet['user']['screen_name'];
                $tweets[$i]['url']          = $tweet['user']['url'];
                $i++;
            endforeach;
        endif;

        $jsonified = json_encode($tweets);  
        $jsoncache = $file_loc;
        $fp = fopen($jsoncache, "w");
        fwrite($fp, $jsonified);
        fclose($fp);    
    endif;

}

Are there any improvements I can make to my code? Eventually I will implement this into a WordPress Plugin. I am curious about any better approaches than fwrite and fclose so if there is a better way to write to a JSON file that would be helpful.

Answer 1

This is a partial review.

$file_loc and $json_folder

Use $json_folder in the construction of $file_loc. If you decide to use a different folder, you need change two variables.

$file_loc = "{$json_folder}/twitter.json";

Naming of function

You named your function get_twitter_json()... which... does essentially nothing. It does not return json like the function name suggests. It doesn't even return the twitter result. Instead this seems a partial implementation of a processing function. If it is supposed to return tweets, then name at least something like get_tweets().

Whitespace

Use consistent whitespace. Trying to align all assignments to the same indentation will create problems sooner than later. Changing one assignment will mess up the rest. Multi-line assignments will look even weirder. I would recommend 1 space before the assignment operator (=) and one space after it.

$results and $tweets

The if-else-statement should have some code behind it. The if-statement loads the decoded json-file in $results, while the else-statement loads it in $tweets. Use one of them.

While you are at it, while php allows you to use $tweets undefined like that, it doesn't make for clear code.

foreach with a counter

You are using a foreach loop with a custom counter. That... defeats the purpose of a foreach loop. Either use a for loop and use the exposed counter from that, or use the append syntax to don't need a counter.

foreach($results as $tweet) : 
    $new_tweet = array();
    $new_tweet['created']      = $tweet['created_at'];
    $new_tweet['text']         = $tweet['text'];
    $new_tweet['username']     = $tweet['user']['name'];
    $new_tweet['screenname']   = $tweet['user']['screen_name'];
    $new_tweet['url']          = $tweet['user']['url'];
    $tweets[] = $new_tweet;
endforeach;

Answer 2

This function is doing WAY too much.

• you are storing unnecessarily hard-coded default configuration
• storing OAuth configuration (where it is not clear how you handle OAuth for individual end users)
• implementating JSON caching mechanism
• interacting with Twitter API
• making (seemingly) trivial transformation on the data returned from API

---

You should STRONGLY consider taking an object-oriented approach here. I think that, because your function is doing too much, you have great opportunity to refactor this code into appropriate classes/objects which each handle a slice of the overall behavior.

---

You should also strongly consider not hard-coding values that should probably be configurable by the caller. Why would you build a function that could only get tweets for a single user name? Why would you not pass in values such as username, number of tweets to retrieve, where the caching directory should lie, etc.?

---

You need to think about interacting with the caller in a consistent way. Let's consider this code:

$some_result = get_twitter_json();

Right now when I call get_twitter_json(), several things can happen:

• You will output to standard out around directory existence/non-existence. Why is this function outputting directly at all? There is nothing in function name that would suggest to caller that it could generate output. If you have a failure scenario, you should probably log an error and then return a meaningful value to caller to indicate failure or throw an exception depending on how you want to the handle the error.
• You potentially echo a very generic and probably useless error message around a problem making the request. You should log errors, not output them. Let the caller decide how to handle the messaging to this user, not this function. Again, why would this function possibly produce spurious output to standard out? You are working with a known API, this code should likely handle ALL known error codes/messages that this API generates and perform appropriate error logging or throwing of exception depending on how you are expecting the caller to handle the various flavors of error (i.e. are they recoverable?).
• If the operation is successful, either via cache retrieval or API call, the function just silently completes, nothing is returned to the caller to indicate success.
• So this brings me back to that example code. When caller is invoking this function, what would you expect to be returned to populate $some_result? Right now, absolutely nothing is returned, no errors logged, no exceptions thrown to indicate to the caller whether the intended action was successful or not. When contrasting this behavior with the function name of get_*, I cannot understand as a caller what I should expect to happen. Typically methods prefixed with get actually return something or a value like false/null when the operation fails. I think you have a poorly named function that doesn't express what is actually happening maybe something like persist_json_from_twitter_feed() would be more meaningful.

---

I honestly don't understand the branch of logic here which reads from cached representation. Why bother even doing that if you aren't actually returning the JSON? Even the branch where you interact with the Twitter API does not return anything to the caller (see earlier comment), but does write to the cache. Is this really just a function to update the cache from the twitter feed? If so, why so you ever need to read the existing cache file and json_decode() it?

---

Stylistically, you have some problems. I think the answer given by @Sumurai8 addresses several of these. I would add that you have no meaningful comments. Oftentimes, well-written code with meaningful variable, class, function, etc. names only needs minimal commenting around logical considerations. Here you lack both comments and well-chosen function/variables names.

---

        $twitter->setGetfield($getfield) 
                ->buildOauth($url, $requestMethod) 
                ->performRequest()

I am guessing that this bit of code (which is inserted inside a json_decode() call) likely has potential need for you to provide some error handling. Does this class throw exceptions such that this code should be within try-catch block? What happens if Oauth fails? What happens if request itself fails?

Your code really only covers "happy path" use cases and is likely to be significantly fragile.