Image Upload Class

Question

This file takes a user picture upload and converts two pictures which are named according to the Session ID.

Right now he biggest drawback is that you cannot crop the image at upload time like you can on popular sites like Facebook.

However I'm just looking for overall constructive criticism on the structure of the class. The paths to save the files to are constants.

I thought about breaking it up in to two classes but I want to stick with just one.

 <?php

/**
 *      Module  :       Model
 *      Name    :       Upload
 *      Input   :       File Information
 *      Output  :       Resized Files in .jpg format
 *      Notes   :

 resizeMove() - resizes the picture to $maxMedium and $maxSmall and moves the file to a permanent location.  
 makeDimensions() - calculates the dimensions of the new images so that there is not distortion if possible.
 getImage() - creates a php image for manipulation.
 updateSessionAndDb - updates the mysql table - move out.

 */

    class Upload
    {
        private $originalWidth, 
                $originalHeight, 
                $newWidth, 
                $newHeight, 
                $maxMedium = 50,
                $maxSmall = 20;

        private $src = NULL;

        private 
                $fileType,
                $fileName,
                $sessionId,
                $path_medium,
                $path_small;

        function __construct($fileType, $fileName)
        {

            $this->sessionId = Session::get('id');
            $this->path_medium = Constant::PICTURES . "$this->sessionId.jpg";
            $this->path_small = Constant::PICTURES . "$this->sessionId-1.jpg";
            $this->fileType = $fileType;
            $this->fileName = $fileName;

        }

        private function createImages()
        {
            if(move_uploaded_file($this->fileName, $this->path_medium))
            {
                if($this->getImage($this->path_medium))
                {
                    list($this->originalWidth,$this->originalHeight)=getimagesize($this->path_medium);
                    $this->resizeMove($this->maxMedium,$this->path_medium);
                    $this->resizeMove($this->maxSmall,$this->path_small);
                    imagedestroy($this->src);
                }
            }
        }

        private function resizeMove($max, $path)
        {
            $this->makeDimensions($max);
            $image_true_color = imagecreatetruecolor($this->newWidth, $this->newHeight);
            imagecopyresampled($image_true_color, $this->src, 0, 0, 0, 0, $this->newWidth, $this->newHeight, $this->
            originalWidth, $this->originalHeight);
            imagejpeg($image_true_color, $path);
            imagedestroy($image_true_color);
        }

        private function makeDimensions($max)
        {
            $this->newWidth=$this->originalWidth; 
            $this->newHeight=$this->originalHeight;
            if(($this->originalWidth > $this->originalHeight) && ($this->originalWidth > $max))
            {
                $this->newWidth = $max;
                $this->newHeight = ($max / $this->originalWidth) * $this->originalHeight;
            }
            elseif($this->originalHeight > $this->originalWidth && $this->originalHeight > $max)
            {
                $this->newHeight = $max;
                $this->newWidth = ($max / $this->originalHeight) * $this->originalWidth;
            } 
            elseif ($this->originalWidth > $max)
            {
                $this->newWidth = $this->newHeight = $max;
            }
        }

        private function getImage($path)
        {
            $type_creators = array( 
                'image/gif' => 'imagecreatefromgif', 
                'image/pjpeg' => 'imagecreatefromjpeg', 
                'image/jpeg' => 'imagecreatefromjpeg', 
                'image/png' => 'imagecreatefrompng'); 
            if(array_key_exists($this->fileType, $type_creators)) 
            { 
                $this->src = $type_creators[$this->fileType]($path); 
                return true; 
            }
        return false; 
        }
    }

Answer 1

With this piece of code you are almost beginning to write good OO. Its good to see you are trying to hide implementation details from other classes.

However, what you have here is a constructor that does everything. The constructor should do no more than initialize the object. Have faith that once you have created your object you can then call its public methods to let the object do what needs to be done.

Think about what an "Upload" object is? The resize move could be a good interace to the rest of the system (that is the essence of uploading)? What else should an "Upload" object be responsible for? Make those things that the object should be responsible for public methods so that other objects can call them.

Updating the session here seems a little odd is that part of uploading?