Jump to content

Is there a neat,clean way to avoid code repetition like this?


MelodyMaker

Recommended Posts

Hi, I would like to know if there's a way to avoid bad repetition like this one:

I have a User class:

class User{
        private $conf;

private $photos 		= Array();
private $addresses 	= Array();
private $descriptions 	= Array();
        ...
        public function __construct($userData){		

		$this->conf			= Configuration::getInstance();
                        ...
        }
        
        public function addPhoto(Photo $photo){		
	$photoNumber = $this->getPhotosAmount();

	if( $photoNumber < $this->conf->userMaxPhotos){
		$photo->setIdUser($this->u_idUser);			
		$photo->setNumber(++$photoNumber);
		$this->photos[] = $photo;
		return true;
	}
	return false;
}

public function addDescription(Description $description){		
	$descNumber = $this->getDescriptionsAmount();

	if( $descNumber < $this->conf->userMaxDescriptions){
		$description->setIdUser($this->u_idUser);			
		$description->setNumber(++$descNumber);
		$this->descriptions[] = $description;
		return true;
	}
	return false;
}

public function addAddress(Address $address){		
	$addressNumber = $this->getAddressesAmount();

	if( $addressNumber < $this->conf->userMaxAddresses){
		$address->setIdUser($this->u_idUser);			
		$address->setNumber(++$addressNumber);
		$this->addresses[] = $address;
		return true;
	}
	return false;
}	


        ...


}

 

 

 

Of course, there are also three "remove" functions. It looks like very bad, to me...

As you can see, the methods are very similar and this is very annoying.

So, I decided to create and abstract class called "profileItem" which is a superclass of Photo, Description, and Address Class.

 

The abstract ProfileItem Class:

 

abstract class AbsProfileItem{

private $number		= 1;
private $idUser 	= 0;

public function getIdUser(){
	return $this->idUser;
}
public function setIdUser($idUser){		
		$this->idUser = $idUser;				
}	
public function getNumber(){
	return $this->number;
}
public function setNumber($number){		
	if($number > 0){	
		$this->number = $number;
	}				
}
}

 

The Photo class:

 

class Photo extends AbsProfileItem {

private $url		= "";
private $caption	= "";	

public function __construct($url,$caption){

	$this->url 			= $url;
	$this->caption 		= $caption;			
}

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

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

}

 

Address and Descriptions are similar...

 

The problem is that I don't know how to integrate them with the user class...I mean, I need, for example an addItem method which can be used for Description, Photo, and Address.

But at some point, there will be a conditional statement, something like:

if ($item instance of "Address") then "the address should be added to $this->addresses

if ($item instance of "Photo") then "the photo should be added to $this->photos

if ($item instance of "Description") then "the description should be added to $this->descriptions

...

 

Is there a clean, elegant way to do this (maybe with the help of a pattern...don't know)?

 

Thank you very much in advance!

 

Davide

 

 

 

 

 

 

 

Link to comment
Share on other sites

Well, I guess now you know why we kept telling you to take these methods out of the User class where they do not belong :)

 

Address and Descriptions are similar...

 

The problem is that I don't know how to integrate them with the user class...I mean, I need, for example an addItem method which can be used for Description, Photo, and Address.

But at some point, there will be a conditional statement, something like:

if ($item instance of "Address") then "the address should be added to $this->addresses

if ($item instance of "Photo") then "the photo should be added to $this->photos

if ($item instance of "Description") then "the description should be added to $this->descriptions

...

 

Is there a clean, elegant way to do this (maybe with the help of a pattern...don't know)?

 

That would be terrible and illegible. What you're looking, honestly, is to create a few Model[1] Objects.

 

<?php

class UserPhotos {
   private $description = null;
   private $photo = null;
   private $address = null;

   public function setAddress($address) {
       $this->address = $address ;
   }

   public function addPhoto(Photo $photo) {
       $this->photos[] = $photo; //only on display should you care about limiting the amount of photos a user can have, or when you try to save. The responsibility doesn't need to be here.
   }

   public function setDescription(array $description) {
        $this->description = $description;
   }

}

 

[1]: http://en.wikipedia.org/wiki/Model_Object

Link to comment
Share on other sites

I'm sorry, I don't want to bother you with my questions but a User has:

 

-a name

-an email

- a country ID

...

 

as private properties :

 

Class User{

 

private name;

private email;

private countryID;

...

 

}

 

so, why it should not contain an array of photos and addresses as properties as well?

Is it because they are arrays?

 

 

 

 

 

Link to comment
Share on other sites

I'm sorry, I don't want to bother you with my questions but a User has:

 

-a name

-an email

- a country ID

...

 

as private properties :

 

Class User{

 

private name;

private email;

private countryID;

...

 

}

 

so, why it should not contain an array of photos and addresses as properties as well?

Is it because they are arrays?

 

 

 

 

 

 

I didn't say a User could not have Photo's, I said that operations and data associated with photos should belong to photos.

Link to comment
Share on other sites

So the operations such as:

 

-add

-remove

-move (by "moving", I mean that the photo number 7 can be moved to position 1 or 2 and the other photos (their numbers) must be ordered consequently.

-getPhotoByNumber (which returns a Photo instance, by passing it his number)

 

Should be left out of the user class?

 

But then, should the user class store an array of photos or not? Since Photos are associated to User by a idUser property, they can be left out. Or am I wrong? From a logical point of view it seems also correct to me to store them in the user object since a foto belongs to a user...

 

 

Link to comment
Share on other sites

Maybe I'm looking at this wrong, but isn't this a clear case of where delegation should be used?  It seems like the most obvious solution to me.

 

I mean, MM wants a User to own (contain) Photos associated with them.  This User, logically, should be able to modify their own Photos.  Why not just have an array of Photos as a property of the User class, then a series of methods that delegate to the Photo's methods?  Something like:

class User
{
   private $photos = array();
   .
   .
   .

   public function addCaption($key, $caption)
   {
      $this->photos[$key]->addCaption($caption);
   }
}

class Photo
{
   private $caption;
   .
   .
   .

   public function addCaption($caption)
   {
      $this->caption = $caption;
   }
}

$Bob = new User();
.
.
.

$Bob->addCaption("vacation", "My summer vacation"); //remember, array keys (in this case, "vacation") can be strings

Link to comment
Share on other sites

Why do you need to add/remove photos? Just set the photo list once it's been open:

 

<?php

class User {

   private $photos = null; // this could also be PhotoList (or Gallery) if you want to have operations done on an entire set of photos. That's what I would recommend.
   private $userId = null;

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

}
?>

Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.