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

[MelodyMaker]

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


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 classes 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

 

[Prattaratt]

Having read your previous post, A couple of questions pop to the forefront of my brain:
1. Is Description supposed to be a description of the photo? if so, you might want to consider making it a property of your Photo Class, instead of storing in a separate array.
2. Why are the Individual Profile Items tracking their counts? Each individual photo should not know or care how many photos are in each profile; same with addresses and descriptions. That responsibility should rest with the user class; it cares and has a need to track such info.
3. I would suggest you look into polymorphism to answer your question. In other words, You are storing ProfileItems, right? Not sure how PHP handles such matters (I use C++ myself) but it seems to me you could redesign your classes as such:

//User Class
//pseudo code, not any particular language, OK?
Class User
{
  integer photoCount = 0;
  integer DescrCount = 0; 
  integer AddrCount = 0;
  Array of ProfileItems Items[];

//Functions
  User()  //Constructor

  Boolean AddProfileItem(ProfileItem Item)
  {
  integer Index = PhotoCount + DescrCount + AddrCount;
  //assuming arrays are zero based; if 1 based, add one to above
    Items[Index] = Item;
    Case Item->Type() of
        1:  Increment PhotoCount;
        2:  Increment DescrCount;
        3:  Increment AddrCount;
  }  // end of AddProfileItem Function

  ~User()  //Destructor   
}

And Now your Base Class:

Class ProfileItem
{
  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;
    }                
  }
// not sure how PHP implements virtual functions so here goes
    virtual public function type() {return 0};
 // this would return 1 for photos, 2 for descriptions and 3 for addresses

}

and then your 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;                
    }  
    virtual public function type() {return 1};
    // overides type function in base class to return 1 for photo
}

I hope this gives you some ideas.

 

[MelodyMaker]

Thanks for you reply.
Description is NOT a description of the photo. It is a description of the user. It can be anything (a description of the user himself or services he offers) and a user can have 0 or more descriptions.

In another post, I was suggested to introduce in the system a PhotoManager in order to do all the database operation such as loading photos,deleting,...

Now, my system should do the same thing for the description, and for the addresses (a user can have many addresses. I consider an email and a phone, an address).

Now, I modeled these objects. Any of these objects belongs to a User.And all of these objects must support the following operation:

-add
-delete
-move (for "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.

Now, some questions:

1) WHERE do these Methods should be put? In the user class? Someone in this forum suggested me so. (see the POST "A question about design: user class (and photo class?)")

2)Now, let's suppose that I need a method such "getPhotobyNumber", which should return me an instance of a Photo object, by passing it his number. Should this method belong to user class as well?

At this point is really necessary to store those objects in the User? Couldn't they be left out of the user class (since those objects maintain a reference (idUser) to the user)?

These design problems are making me mad...

Thanks for your help!

 

[jmeckley]

The "other" objects should not hold a reference to the userId, they should hold a reference to the User object.

getPhotobyNumber should be placed on the aggregate (root) object. It depends on the context you are using. For more information about Aggregates, contexts and such check out Domain Drive Design by Evans. This book would help you solve the problems you are currently facing.

Jason Meckley
Programmer
Specialty Bakers, Inc.

 

[MelodyMaker]

Thanks Jason! I found that book in the Internet. I will buy it as soon as possible