4
\$\begingroup\$

What I'm doing

Using the v3 of the Wufoo API to retrieve all entries and store in a single array ($all_results). This is necessary because of the limit 100 entries limit that Wufoo has when retrieving results from their api. Also store the results to file we don't get slow responses from the api.

My question

I'm wondering how I could have done this better. This code is working for me, but I want to get better.

<?php

function wufoo_entries() {
    // Setup
    $account = "ACCOUNT_NAME";
    $form = "FORM_NAME";
    $api_key = "YOUR-API-KEY";
    $offset = 0;
    $limit = 100;
    $check_int = 15 * 60; // 15 minutes
    $file_time = 'time_stamp.txt';
    $file_results = 'results.txt';


    // Check for the last update
    $time = time();
    $time_stamp = file_get_contents($file_time);
    $refresh = ($check_int < ($time - $time_stamp)) ? TRUE : FALSE;

    // if true run functions and save the results and time to file else pull the results from file
    if ($refresh) {
        // Initialize our results array
        $all_results = array();

        // How many entries are there?
        $api_uri = "https://$account.wufoo.com/api/v3/forms/$form/entries/count.json";
        $count = wufoo_api($api_key, $account, $form, $offset, $limit, $api_uri);
        $counted = $count['EntryCount'];

        // Call the api as many times as needed
        for ($offset = 0; $offset <= $counted - 1; $offset = $offset + $limit) {
            $results = wufoo_api($api_key, $account, $form, $offset, $limit);
            // Limit the number of times we put results into the $all_results array to make sure we don't end up with blanks
            $limit = (($offset + $limit) > $counted) ? $limit - (($offset + $limit) - $counted) : $limit;

            // Iterate through the Entries array to push each entry into a new array
            for ($result = 0; $result <= $limit - 1; $result++) {
                array_push($all_results, $results['Entries'][$result]);
            }
        }

        // Save the new results (as json) and time to file
        $all_results_file = json_encode($all_results);
        file_put_contents($file_results, $all_results_file);
        file_put_contents($file_time, $time);
        return $all_results;
    }

    else {
        // Pull the results array from file
        $all_results = file_get_contents($file_results);
        $all_results = json_decode($all_results, true);
        return $all_results;
    }
}

function wufoo_api($api_key, $account, $form, $offset, $limit, $api_uri = NULL) {
    // Check to see if it's asking for the the entries or the total count
    $api_url = ($api_uri) ? $api_uri : 'https://' . $account . '.wufoo.com/api/v3/forms/' . $form . '/entries.json?pageStart=' . $offset . '&pageSize=' . $limit;

    $curl = curl_init($api_url);
    curl_setopt($curl, CURLOPT_RETURNTRANSFER, 1);
    curl_setopt($curl, CURLOPT_USERPWD, $api_key . ':footastic');
    curl_setopt($curl, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
    curl_setopt($curl, CURLOPT_SSL_VERIFYPEER, false); curl_setopt($curl, CURLOPT_FOLLOWLOCATION, true); curl_setopt($curl, CURLOPT_USERAGENT, 'Wufoo Sample Code');
    $response = curl_exec($curl);
    $resultStatus = curl_getinfo($curl);

    if ($resultStatus['http_code'] == 200) {
        $results = json_decode($response, true);
        return $results;
    } else {
        $results = 'Call Failed '.print_r($resultStatus, true);
        return $results;
    }
}

// Show me some results and make sure it works
echo "<pre>";
print_r(wufoo_entries());
echo "</pre>";

?>
\$\endgroup\$
  • 1
    \$\begingroup\$ Would you consider turning your code into a class? It's ok if you don't... \$\endgroup\$ – yannis Nov 15 '11 at 7:45
  • \$\begingroup\$ That what I was thinking too, thanks for the input. \$\endgroup\$ – michaelespinosa Nov 15 '11 at 20:54
2
\$\begingroup\$

Just some quick note:

1, I would separate the configuration settings to a distinct php file (and use a WufooConfig class/struct) and include it at the beginning of the code. It makes the configuration easier to change. If you use a WufooConfig class/struct and pass it to the wufoo_entries() function you will be able to use the same function to fetch data from more than one accounts. Furthermore, it would encapsulate configuration, so you just have to pass around one parameter instead of $account, $form, $api_key, etc.

class WufooConfig {
    private $account;
    private $form;
    private $api_key;
    ...

    public function __construct($account, ...) {
        $this->account = $account;
        ...
    }
}

2, Extract the then and else branches of the if in the wufoo_entries() function:

if ($refresh) {
    $results = fetchContent(...);
    saveToCache($results, ...); // write file here
    return $results
} else {
    return getCachedContent();
}

It makes your code easier to read, you'll see the big picture immediately, it won't be distracted with low-level API calls.

For the same reason I'd create some new functions, for example:

functio entryCount(...)
    $count = wufoo_api($api_key, $account, $form, $offset, $limit, $api_uri);
    $counted = $count['EntryCount'];
    return $counted;
}

The name of the function says what you want to get and the body shows how you do it.

3, Change

for ($offset = 0; $offset <= $counted - 1; ...

to

for ($offset = 0; $offset < $counted;

It's more common.

4, In the inner for loop the $result loop variable is a little bit confusing, it's easy to mix up with $results. $resultIndex would be better.

for ($resultIndex = 0; $resultIndex < $limit; $resultIndex++) {  
    array_push($all_results, $results['Entries'][$resultIndex]);
}

Finally, I agree with @Yannis Rizos, the whole logic would be better as a class.

\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks palacsint for the great feedback. I'll work on splitting the code up into more logical and easy to read chunks and put it into a class. I appreciate the input, thanks! \$\endgroup\$ – michaelespinosa Nov 15 '11 at 20:56

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.