2
\$\begingroup\$

I've prepared a function using cURL to get a remote file to the local storage:

/**
 * cURL - Get Remove File
 * 
 * Function using CURL the get a remote file to the local storage.
 * 
 * @param str $url              Full remote URL
 * @param str $xml_path         Internal path to the storage the file
 * @param str $xml_file         Filename of the xml file to save
 * @param arr $access           Access credentials (usr;pwd)
 * 
 * @return bollean
 */
function get_remote_file($url, $xml_path, $xml_file, $access = null) {

  $curl_handle = curl_init();
  curl_setopt($curl_handle, CURLOPT_HEADER, 0);
  curl_setopt($curl_handle, CURLOPT_URL, $url);
  curl_setopt($curl_handle, CURLOPT_CONNECTTIMEOUT,2);
  curl_setopt($curl_handle, CURLOPT_RETURNTRANSFER,true);
  if ($access) {
    curl_setopt($curl_handle, CURLOPT_USERPWD, $access["usr"].":".$access["pwd"]); 
  }
  curl_setopt($curl_handle, CURLOPT_SSL_VERIFYPEER, false);
  $buffer = curl_exec($curl_handle);
  curl_close($curl_handle);

  if ($buffer != false) {
    file_put_contents($xml_path.$xml_file, $buffer);
    return true;
  }

    return false;

}

While is working fine, even with password protected files, I'm not too comfortable with CURL, so I wonder if this can be optimized or improved.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I'm not very familiar with cURL either, but looking at the documentation I noticed a curl_setopt_array() function that may be worth looking into.

$options = array(
    CURLOPT_HEADER => FALSE,
    CURLOPT_URL =>  $url,
    //etc...
);

curl_setopt_array( $curl_handle, $options );

Looking at the documentation for curl_exec() shows that the only time its output will be FALSE is on failure. The only time it is really necessary to compare a value explicitly with a boolean is if that value will at some point be a boolean and at all other times won't, but may be mistaken for it (0, '', 'false', etc...). However, at this point you would then use absolute equality comparison instead of a loose one. For instance, the below example will output "asdf", "false", '', and 0 from the first if statement, but the second and third if statements will only ever output "asdf". This is because the first if statement uses an absolute, or strict, comparison to the boolean. The third if statement shows a cleaner way of doing the second. Any variable you can loosely compare with a boolean, can loosely be used as a boolean, so there's no need to type out the actual comparison. I would use that last if statement because its not likely you want to create an empty file, and unless you have a file with only a "false" value in it, then you are not likely to notice the difference.

$buffer = array( FALSE, 'asdf', 'false', '', 0 );

foreach( $buffer AS $test ) {
    if( $test !== FALSE ) {//absolute
        var_dump( $test );
    }
    if( $test != FALSE ) {//loose
        var_dump( $test );
    }
    if( $test ) {//another way of writing loose
        var_dump( $test )
    }
}

The only other thing I would suggest here is that you verify that your $xml_file exists before trying to write to it.

if( $buffer && is_file( $xml_path . $xml_file ) ) {
\$\endgroup\$
1
  • \$\begingroup\$ Thank you for the analisys, I'll be looking into this as to ascertain the improvement :) About the file_put_contents, the goal is actually to create or overwrite the existent file, hence the reason as to why I'm not confirming if it exists, leaving the default action of this function taking place. +1 \$\endgroup\$
    – Zuul
    Sep 18, 2012 at 9:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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