A question about design: user and photo classes

[MelodyMaker]

Sorry if I bother you with this, but I'm a OOP newbie and I would like to know your opinion about that.

 

I'm developing a site which requires registration and where users can upload their fotos. The question is this:

 

 

I created a user class

 

class User{
private $username;
private $email;
private $country;
???private $photos= Array();???
...
function __construct($idUser)
//here I load the details about the user;
}

 

Now, every user can upload at most 12 photos, which can have a small caption.

So, the question:

 

1) Do I need a "Photo" Class? (the encapsulation principle tells me so)

In this case what is the best way to associate this class to the user class? By storing the photo objects in an array property of the user class? And where should I put the method "loadPhotos"? In the user or in the photo class?

 

class Photo(){
private $url;
private $caption;

function __construct($IDPhoto){
.... here another select on the same table, mmm
}
}

 

...but if I do this, there will be two SELECT : One to get the user's ID photo and the other to retrieve photo's data. Or I'm wrong..?

 

 

2) Storing all the data about photos in an array (as a property) and forget about Photo class?

 

I'm a bit confused, sorry.

 

Thank you very much in advance.

 

Davide

[maexus]

A user's object should only have to worry about user's information. Not the uploading, editting or viewing of photos. If the photos are stored in a gallery format, the galleries may have an id that can be stored in a user's object, just for reference. Then you could have a gallery object for managing and organizing photos and a photos object for editting, outputing the images.

 

You should always try to break down resposibilities are far as possible within reason (some may argue that)

[MelodyMaker]

Thank you!

 

The fact is that I really can't figure out, how the two classes are associated, I mean:

let's suppose that I give the user (it is so) the ability to add (upload) a photo and delete a photo.

Then, according to what you say, these methods should be part of the Photo Class.

 

which parameters should I pass to the AddPhoto? A reference to the user that owns that photo. So, first I have to instantiate User class, then the foto class (what about the parameters in its constructor?) and then, somewhat pass the userId to the photo Class?

 

Please not that my photo has only four attributes: a url, a caption, an idUser and a number (plus the methods to upload and delete, if I put them here).

 

Sorry, I know that I'm terribly confused...

[KevinM1]

Thank you!

 

The fact is that I really can't figure out, how the two classes are associated, I mean:

let's suppose that I give the user (it is so) the ability to add (upload) a photo and delete a photo.

Then, according to what you say, these methods should be part of the Photo Class.

 

which parameters should I pass to the AddPhoto? A reference to the user that owns that photo. So, first I have to instantiate User class, then the foto class (what about the parameters in its constructor?) and then, somewhat pass the userId to the photo Class?

 

Please not that my photo has only four attributes: a url, a caption, an idUser and a number (plus the methods to upload and delete, if I put them here).

 

Sorry, I know that I'm terribly confused...

 

Remember, objects can contain other objects.:

<?php
   class User
   {
      private $userId;
      private $userName;
      private $email;
      private $country;
      private $photos = array();

      public function __construct($userId, $userName = null)
      {
         $this->userId = $userId;
         $this->userId = $userName;
      }

      public function addPhoto($url, $caption)
      {
         $photo = new Photo($url, $caption); //create a new photo
         $photo->upload($this); //upload it
         $this->photos[] = $photo; //add it to the user's array of photos
      }

      public function setId($id)
      {
         $this->userId = $id;
      }

      public function getId()
      {
         return $this->userId;
      }
   }

   class Photo
   {
      private $url;
      private $caption;

      public function __construct($url, $caption)
      {
         $this->url = $url;
         $this->caption = $caption;
      }

      public function upload(User $user)
      {
         $userId = $user->getId();

         $query = "INSERT INTO photos (url, caption) VALUES ({$this->url}, {$this->caption}) WHERE user_id = '$userId'";
         $result = mysql_query($query);

         if($result)
         {
            //success!
         }
         else
         {
            //failure
         }
      }
   }

   $Bobby = new User(23);
   $Bobby->addPhoto("vacation.jpg", "My summer vacation");
?>

[MelodyMaker]

THANK YOU VERY MUCH!!!!

[MelodyMaker]

Just a quick question:

 

passing a User instance as argument to the upload photo method, is it correct from a formal point of view?

[KevinM1]

Just a quick question:

 

passing a User instance as argument to the upload photo method, is it correct from a formal point of view?

 

Yup, it's fine.  Despite the need for encapsulation, objects also still need to communicate with each other.  This is done by passing objects as arguments to another object's methods.  The way I did it above - with the object passing itself as an argument to a function - is often a sign of delegation.  You'll see it many more times as you get more comfortable with OOP.

[MelodyMaker]

Thanks again, and I'm sorry if I ask you another question, but this is very very important for me. There's still something I don't understand.

 

You wrote the photo Class:

 

class Photo
   {
      private $url;
      private $caption;

      public function __construct($url, $caption)
      {
         $this->url = $url;
         $this->caption = $caption;
      }

      public function upload(User $user)
      {
         $userId = $user->getId();

         $query = "INSERT INTO photos (url, caption) VALUES ({$this->url}, {$this->caption}) WHERE user_id = '$userId'";
         $result = mysql_query($query);

         if($result)
         {
            //success!
         }
         else
         {
            //failure
         }
      }
   }

 

The constructor gets a url and a caption as arguments. That's ok. What is making me mad, now, is that I would like to have a method that gives me all the photo for a specific user and also a metod to delete a photo.

 

The first problem is: where should I place those methods?

Something tells me that they should be in the user Class:

 

class User{
...

public function getAllPhotos(){

$QUERY = "SELECT * FROM userphoto WHERE idUser=..."
$result = mysql_query($QUERY);

while($res = $result->fetch_array()){
$photo = new Photo($res['url'],$res['caption'])
$this->photos[] = $photo
}

}

}

 

Is this correct?

Now the problem: The delete method.

It should not only remove a photo from the "photos" array of the user class, but also delete it from database (and maybe even delete it from disc). For this reason, I think that it would be nice to have also a IDPhoto value somewhere (which is infact the primary key value of the table userphoto).

Otherwise, the SQL DELETE statement would be really bad:

DELETE FROM userphoto WHERE idUser= and url=... 

best would be:

DELETE FROM userphoto WHERE IDPhoto = $IDPhoto.

 

I don't know how to insert IDPhoto, since we construct the class only with url and caption.

 

And last, I will certainly use also a "number" field to number the photos, a user has. I don't know if this could be use to delete the photos, instead of using a IDPhoto..

 

Thanks in advance, I know I'm asking you a lot.Sorry if I bother you...

 

Davide

 

 

 

 

 

 

 

 

 

 

 

[KevinM1]

Well, the first thing to keep in mind is that nothing is set in stone.  If you need another parameter, or method, then feel free to add it.  One of the things I used to get stuck with was looking at examples (such as the ones other people here would give me) as though they were hard rules to follow.  They're not.  They're just an illustration on one way of approaching a problem.  The biggest thing to get is the concepts behind the code examples.

 

Okay, now to the code.

 

A method to get all photos for a particular user is very simple.  Just return the photos array:

<?php
   class User
   {
      .
      .
      .

      public function getPhotos()
      {
         return $this->photos;
      }
   }

   $Kevin = new User(15);

   $Kevin->addPhoto("vacation.jpg", "My summer vacation");
   $Kevin->addPhoto("dog.jpg", "My dog");

   $kevPhotos = $Kevin->getPhotos();

   foreach($kevPhotos as $photo)
   {
      echo "<img src='{$photo->getUrl()}' alt={$photo->getCaption()}' /><br />";
   }
?>

 

Deleting a photo is a bit more work, but still not too complicated.  The first step is in finding the photo we want to delete.  The second is in deleting it from the database.  The final step is removing it from the user.

<?php
   class User
   {
      .
      .
      .

      public function deletePhoto($photoId)
      {
         $tmp = array(); //temporary array to hold non-deleted photos

         for($i = 0; i < count($this->photos); i++) //loop through all the photos for this user
         {
            if($this->photos[$i]->getId() == $photoId) //if it matches the one we want to delete, store it in the $delPhoto variable
            {
               $delPhoto = $this->photos[$i];
            }
            else //else, add it to the temporary array
            {
               $tmp[] = $this->photos[$i];
            }
         }

         $this->photos = $tmp; //set the user's array to the temporary array...if we have a photo to delete, it WON'T be included because we already stored it in $delPhoto

         if($delPhoto) //if there's a photo to delete
         {
            $delPhoto->deletePhoto(); //delete it
         }
      }
   }

   class Photo
   {
      private $photoId;
      .
      .
      .

      public function __construct($photoId, $url, $caption)
      {
         $this->photoId = $photoId;
         $this->url = $url;
         $this->caption = $caption;
      }

      public function getId()
      {
         return $this->photoId;
      }

      public function getUrl()
      {
         return $this->url;
      }

      public function getCaption()
      {
         return $this->caption;
      }

      public function deletePhoto()
      {
         $query = "DELETE FROM userphoto WHERE IDPhoto = '{$this->photoId}'";
         $result = mysql_query($query);
      }
   }
?>

 

If you need the userId to delete a photo, then just pass the deletePhoto() method the user object, just like we did with addPhoto.

[MelodyMaker]

Thanks for your answer!

 

The getPhotos method you wrote works only if first a user adds a photo. But sometimes, I have to load all the photos from the database, without using the method add. This is this point my question came from:

 

function getPhotos(){

 

SELECT * from userphoto where idUser = ...

 

//at this point, I have to build the object photo, but how?

while ($res = $result->fetch_array()){

//here I can  get the IDPhoto, since i retrieve it from the DB (the link (url) to the foto is in the DB and the file has uploaded)

$photo = new Photo ($res['IDPhoto'] $res, $res['caption'])

//

}

 

BUT, for example, when a user, uploads a photo (if the upload has success) the photo can't have a IDPhoto,

because it gets (or should get) that ID ONLY after a successful update AND after an INSERT into a DB.

So, a constructor like this:

 

new Photo ($IDPhoto,$url, $caption)

 

in the case of an upload does not work, I think.

 

It is this that is making me crazy...

 

 

 

 

 

}

[KevinM1]

I think you should use the mysql_insert_id() function (http://www.php.net/manual/en/function.mysql-insert-id.php).  It returns the ID if the last INSERT query.  So, you'd upload the raw info first (url, caption), then create a new Photo object using that info and the result generated from mysql_insert_id().

[corbin]

Had a question, but I see you already answered it earlier....  Guess I should read things more closely next time....

 

Original post:

Sorry to intrude in this thread, but I have a coding practice... question....

 

So, if you were to display the images in a gallery....  The only purpose of that page is to display images....

 

Would you load a Photo class for each image just to grab the URL, or would you have a function that would simply fetch all of the image info?  Seems like a lot of overhead just to grab URLs and possibly descriptions?

 

(In this situation it's users, and not outputting to a gallery, but my question is kind of related x.x.)

 

Sorry to go pretty much offtopic, but I figured it might be better than making a new thread....

 

I still wonder though...  Isn't it overkill to spawn a Photo class for each photo just to display them?

[Aeglos]

In the case of methods such as deletePhoto() or findAllPhotos() which seem to be common for all pictures but not really that tied to a User object, isn't it easier (better approach/practice?) to create an object that specifically holds common functionality? Such as a Gallery object, or PhotoManager?

 

Then the specific User object is associated with that particular "manager" object which holds the references to all it's pictures (Photo objects) instead and does tasks such as fetching all photos, deleting photos, finding by id or date or size, perhaps sharing photos across users? sorting, etc.

[MelodyMaker]

Yes, some people told me to go that way as well. So, we should have three classes involved:

 

a Photo Class

a PhotoRepository Class

a User Class.

 

Photo Class which models a photo:

 

Class Photo{
function __construct($url,$number,$caption){
  ...
}
}

 

a PhotoRepository Class which has (or stores) a User reference:

 

Class PhotoRepository{
public function __construct(User $user) //<- is it correct to pass it as a constructor parameter?
{...}
public function getAllPhotos(){
  //Here the query to the DB
  $QUERY = "SELECT * FROM userphoto WHERE idUser='.$this->user->getIdUser()'.";
  ...
$arr = Array(); 
while($res = ...){
   $arr[] = new Photo($res[url], $res['caption'],$res['number']
  }
   return $arr;
}
//What about deleting a photo??? (from the repository AND FROM THE DB?)
}

 

and the class User:

 

class User{
function __construct($id){
...
}
//The following methods should call a PhotoRepository method?
public function Addphoto()
{
  ...
}
public function removePhoto()
{
}

}

 

 

 

[KevinM1]

A PhotoManager class would definitely clean things up (I'm kinda annoyed I didn't think of it myself ;-) ).  You could put the grunt work of dealing with the DB within it.  I'm thinking along the lines of:

<?php
   class PhotoManager
   {
      private $instance;

      private final function __construct(){}

      public static function getInstance()
      {
         if(!self::$instance)
         {
           $this->instance = new self();
         }

         return $this->instance;
      }

      public function getAllPhotos(User $user)
      {
         $userId = $user->getId();

         $query = "SELECT * FROM userphotos WHERE userId = '$userId'";
         $result = mysql_query($query);
         return $result;
      }

      public function deleteAllPhotos(User $user)
      {
         $userId = $user->getId();

         $query = "DELETE FROM userphotos WHERE userId = '$userId'";
         $result = mysql_query($query);
      }

      public function deleteGallery()
      {
         $query = "DELETE FROM userphotos";
         $result = mysql_query($query);
      }
      .
      .
      .
   }

   class User
   {
      .
      .
      .

      private $photos = array();

      public function getAllPhotos()
      {
         $manager = PhotoManager::getInstance();
         $photos = $manager->getAllPhotos($this); //note, $photos != $this->photos
         
         return $photos;
      }

      public function displayAll()
      {
         $photos = $this->getAllPhotos();

         if($photos)
         {
            while($row = mysql_fetch_assoc($photos))
            {
               echo "<img src='{$row['url']}' alt='{$row['caption']}' /><br />";
            }
         }
         else
         {
            echo "No photos found";
         }
      }

      public function deleteAllPhotos()
      {
         $manager = PhotoManager::getInstance();
         $manager->deleteAllPhotos($this);
      }
   }
?>

[MelodyMaker]

...but the AddPhoto and DeletePhoto are still in user class?

 

And your method: 

public function getAllPhotos(User $user)
      {
         $userId = $user->getId();

         $query = "SELECT * FROM userphotos WHERE userId = '$userId'";
         $result = mysql_query($query);
         return $result;
      }

 

This should build and return Photo instances, right?

[KevinM1]

...but the AddPhoto and DeletePhoto are still in user class?

 

And your method: 

public function getAllPhotos(User $user)
      {
         $userId = $user->getId();

         $query = "SELECT * FROM userphotos WHERE userId = '$userId'";
         $result = mysql_query($query);
         return $result;
      }

 

This should build and return Photo instances, right?

 

You could remove addPhoto and deletePhoto from the User class and put them in the PhotoManager.  You'd still have to pass the correct User instance into the method, and, in this case, you'd still need helper methods within the User class itself to add/remove photos from the $photos array.  IMO, both ways seem equally logical on the surface (not looking at the rest of your system), so feel free to use the one you think fits best.

 

Regarding getAllPhotos, I left it rather sparce on purpose.  You can have it create Photo objects and return an array of those.  In fact, you should probably have another method that does that.  I left it like this because I saw it as more of a utility method.  You may only want the raw DB info and not Photo objects.  A better way to do this would be to rename this method something like loadAllPhotos, and have another getAllPhotos method invoke it and use the raw data to create Photo objects.

[keeB]

I have developed something similar, let me share with you how I designed a simple gallery site. Instead of a 'PhotoManager' I used a 'Gallery,' which is kind of the same in principal.

 

A Gallery contains a list of Albums. Albums contain a list of photos. For me, you can choose a Photo to be your Album 'Cover' on display (not really modeled here, but you can see where it would go.)

 

<?php

class Photo {
   private $width;
   private $height;

   public function draw();
   public function resize($width, $height); //wrapper for imagemagick, returns a new Photo object.
   public function getDimensions(); // returns width/height
   public function load($path);
   public function save($path);

}

class Album {
   private $photoList = array(); //contains a list of photos to iterate over in draw();
   public function draw(); // draws the collection of photos
   public function addPhoto(Photo $photo); //add a photo
   public function clear($this->photoList = null;)
}

class Gallery {
   private $album_list = array();
   private $galleryDao;
   public function load(); //use dao to 
   public function draw(); //draws
   public setDao(GalleryDao $dao) {$this->galleryDao = $dao;}
}

class GalleryDao {
//database logic is here
   public function findGalleriesByUser(User $user);
   public function getRandomGallery();
}


class User {
#model object
   private $userId;
   .
   .
   . #some other info, doesn't matter
   
   public function getId();
   public function getName();
}

?>

 

If you have any specific questions, let me know!

[MelodyMaker]

Thank you very much for your suggestions!

 

What I maybe still don't understand (and I hope this is due to the fact that I'm a beginner) is to introduce the Gallery (or PhotoManager object).

 

I mean, why? Loading, adding photos, and so on, cannot be simply methods of the user class? Or this violates the encapsulation principle? Is it for that reason?

[MelodyMaker]

Oh, and someone told me to also introduce into the system a UserManager that creates user instances by passing a $idUser to his method:

 

 

$userManager = UserManager::getInstance();
$user = userManager->getUserById($idUser);

 

I know that this is much cleaner, but is that the only reason to introduce all those "managers"?

 

Thank you very much in advance.

 

Davide

 

[keeB]

Whatever makes sense to you -- go for it.

 

What makes sense to me is to break everything I can up in to very small pieces of responsibility. What this grants me is to make sure every little piece does it's job -- and does it well (aka, the first step to encapsulation.)

 

If you give something too much responsibility, just like in real life, there's some trade off's. Programming/design IS a list of trade off's, and since you're still a beginner they won't be so apparent.

 

Fact is, with your current design you're more than able to finish what you've set to accomplish. I think that's something noteworthy. So go for it! And let us help if needed along the way.

[MelodyMaker]

And let us help if needed along the way.

I'll do it for sure.

Thank you very much to all of you!