How to make a function here

[m8fyu]

Hi

I'm learning PHP but really struggle with functions. I tend to simple bundle all my code into a single script and would like to figure out a little more about functions.

I've used a few online tutorials to make a gallery of images that can be uploaded by users. I'd like to make the bold section into a create_thumbnail function that I can store in my fma_functions.php file - the trouble is I get confused about how to pass in the right arguements. Its currently included as part of my upload.php file

Can anyone help me out please? Any advice would be greatly appreciated.



<?php
// Start a session for error reporting
session_start();

// Call our connection file
require("includes/conn.php");

// Call our functions file
require("includes/fma_functions.php");

// Set some constants

// This variable is the path to the image folder where all the images are going to be stored
// Note that there is a trailing forward slash
$TARGET_PATH = "images/";

// Get our POSTed variables
$image_title = $_POST['image_title'];
$image_descr = $_POST['image_descr'];
$image = $_FILES['image'];
$filename = $image['name'];
$filetype = $image['type'];

// Sanitize our inputs
$image_title = mysql_real_escape_string($image_title);
$image_descr = mysql_real_escape_string($image_descr);
$image['name'] = mysql_real_escape_string($image['name']);

// Build our target path full string. This is where the file will be moved do
// i.e. images/picture.jpg
$TARGET_PATH .= $image['name'];

// Make sure all the fields from the form have inputs
if ( $image_title == "" || $image_descr == "" || $image['name'] == "" )
{
$_SESSION['error'] = "All fields are required";
header("Location: index.php");
exit;
}

// Check to make sure that our file is actually an image
// You check the file type instead of the extension because the extension can easily be faked
if (!is_valid_type($image))
{
$_SESSION['error'] = "You must upload a jpeg, gif, or bmp";
header("Location: index.php");
exit;
}

// Here we check to see if a file with that name already exists
// You could get past filename problems by appending a timestamp to the filename and then continuing
if (file_exists($TARGET_PATH))
{
$_SESSION['error'] = "A file with that name already exists";
header("Location: index.php");
exit;
}

// Lets attempt to move the file from its temporary directory to its new home
if (move_uploaded_file($image['tmp_name'], $TARGET_PATH))
{
// Get image file details
//list function takes data from an array and puts it into the specified variables
if (!list($width,$height,$type,$string) = getimagesize($path_to_images_directory . $filename)) {
echo "Could not access file or file not a valid image.<br />";
}

// Check if image is smaller (in both directions) than required image
// If so, use original image dimensions
// Otherwise, Test orientation of image and set new dimensions appropriately

//*** need to add statements to check if image is smaller = keep original dims ***

$size = getimagesize($path_to_images_directory.$filename); //
//--wide image
if ($size[0] > $size[1]){ //if width is greater than height
$thumbnail_width = 150; //150 is our make dim for our thumbnail
$thumbnail_height = (int)(150 * $size[1] / $size[0]);
}
//--tall image
else {
$thumbnail_width = (int)(150 * $size[0] / $size[1]);
$thumbnail_height = 150;
}
//--create an array that lists the suffix types
//--these will append to the function (imagecreatefrom* and image*)to dynamically build the appropriate function name depending on the filetype
$gd_function_suffix = array(
'image/pjpeg' => 'JPEG',
'image/jpeg' => 'JPEG',
'image/gif' => 'GIF',
'image/bmp' => 'WBMP',
'image/x-png' => 'PNG',
);

//--get the suffix on the basis of the mime type
$function_suffix = $gd_function_suffix[$filetype];

//--build the function name for imagecreatefrom*
$function_to_read = 'imagecreatefrom'.$function_suffix;

//--build the function name for image*
$function_to_write = 'image'.$function_suffix;

//--resizing
//--read the source file
$source_handle = $function_to_read($path_to_images_directory.$filename);

if ($source_handle){
//create a blank image for the thumbnail
$target_handle = imagecreatetruecolor($thumbnail_width, $thumbnail_height);

//resize it
imagecopyresampled($target_handle, $source_handle,
0,0,0,0, $thumbnail_width, $thumbnail_height,
$size[0], $size[1]);
}

//save this thumbnail
$function_to_write($target_handle, $path_to_thumbs_directory.'th_'.$filename);


// we are only going to put the data relating to the full size image here
// this is because we dont want our query to have to read lots of data from our table
// we know that the thumbnail image will be the same filename with '_th' at the end
// therefore when we display our images in our XHTML page we can simply refer to the full-size data from each row and add _th to call thumbnail

// NOTE:
// We are *not* putting the image into the database; we are putting a reference to the file's location on the server
$sql = "insert into images(image_title, image_descr, filename) values ('$image_title', '$image_descr', '" . $filename . "')";
$result = mysql_query($sql) or die ("Could not insert data into DB: " . mysql_error());
header("Location: images.php");
exit;
}
else
{
// A common cause of file moving failures is because of bad permissions on the directory attempting to be written to
// Make sure you chmod the directory to be writeable
$_SESSION['error'] = "Could not upload file. Check read/write persmissions on the directory";
header("Location: index.php");
exit;
}

?>

[rocknbil]

OK . . . well the short answer is if the function exceeds more than 25 lines, it's time to look for bits inside the function you can break off into other functions. The second idea is pass what you need to it, get what you need from it, but most importantly don't allow any hard coding or dependence on external variables inside it, make it a stand alone bit of code. (Sometimes you have to do this, but most of the time you don't.)

First look at how most internal PHP functions work.

if (!list($width,$height,$type,$string) = getimagesize($path_to_images_directory . $filename))...

So you are passing a single parameter and getting an array as a result. Then,

$size = getimagesize($path_to_images_directory.$filename);...

$size will now also be an array, which is why you're doing $size[0], size[1], etc. So approximately the function looks **something** like this.

function getimagesize ($full_file_path) {
// do stuff that creates $w,$h,$type,$some_string
return Array($w,$h,$type,$some_string);
}

A couple things to know about functions you create: variables you define inside the function are not available outside the function, and vice versa. If you wanted to do that you have to use constants or define variables as globals.

define ('AVAILABLE_EVERYWHERE', 'foo');
$bar = 'bar';

Only AVAILABLE_EVERYWHERE will return.

list ($this,$that) = testme();
echo "$this $that";

function testme() {
return Array (AVAILABLE_EVERYWHERE,$bar);
}

You would need to define $bar as a global ...

function testme() {
global $bar;
return Array (AVAILABLE_EVERYWHERE,$bar);
}

... which really should be avoided - you want functions to be self contained bits you can feed parameters to and get results (although, in rare cases it can't be avoided ,or is much more convenient[lazy].) Alternatively there is a $_GLOBALS array you can add stuff to, but you should avoid this if you can, this makes your code reliant on other code somewhere else and very difficult to maintain or modify.

The reason for this preamble is that though you're on the right track, why just the thumbs? Wouldn't something like this be much more cool?

$errors = null;
if (! isset($_POST['photo'])) { // or something else indicating form submit
output_form('Upload Your Photo',null);
// Output form exits, no need for else below
}
list($filename,$errors) = upload_images($_POST['photo']);
if ($errors) { output_form('Error Uploading',$errors); } // which exits
list ($img_url,$errors) = resize_and_thumb($upload_directory,$filename);
if ($errors) { output_form('Error Resizing Image',$errors); } // also exits
output_success_message($image_url);

So now you've four functions that make up your entire program, one for the form output, one to upload, one to resize, one for success (which can be in the same function as the form output.) Or you could nest resize inside upload:

$errors = null;
if (! isset($_POST['photo'])) { // or something else indicating form submit
output_form('Upload Your Photo',null);
// Output form exits, no need for else below
}
list($image_url,$errors) = upload_images($_POST['photo'],$upload_directory);
if ($errors) { output_form('Error Uploading',$errors); } // which exits
output_success_message($image_url);

function upload_images($input,$dir) {
// so you get a full path after moving original.
$upload_errors=null;
// do stuff
list ($preview_path,$resize_errors) = resize($dir,$full_image_path);
if ($resize_errors) { $upload_errors .= $resize_errors; }
return Array ($preview_path,$upload_errors);
}

I can't point specifically how you'd do that with what you have here other than giving you the groundwork, from what I can see you'd have to do a lot of moving around and renaming variables, but it will be a labor well worth the effort. When you start compartmentalizing your code this way, you get away from procedural code and move more toward OOP, and will begin to avoid (what I consider) lazy approaches like this:

$_SESSION['error'] = "All fields are required";
header("Location: index.php");
exit;

I say lazy in that it relies on sessions and redirecting to maintain form/error state. If your code is function oriented, these can be passed as parameters from within the program without redirecting.

[m8fyu]

Thanks for the info. It really helped me out and I managed to get write my create_thumb function and use it from within my upload.php page.