SitePoint Sponsor

User Tag List

Results 1 to 6 of 6
  1. #1
    Twitter: @AnthonySterling silver trophy AnthonySterling's Avatar
    Join Date
    Apr 2008
    Location
    North-East, UK.
    Posts
    6,111
    Mentioned
    3 Post(s)
    Tagged
    0 Thread(s)

    OOP - Object Over-Composition?

    Good morning all,

    I'm working on some hypothetical OO, which I'm currently basing on a Music Chart, and I have some niggling concerns that I hope to gain some feedback for.

    I currently have 3 objects, Song, ChartEntry & MusicChart, for clarity, here's the current code.

    Song
    PHP Code:
    class Song
    {
        private 
    $aData = array();
        
        public function 
    __construct$sArtist $sTitle )
        {
            
    $this->aData['artist'] = $sArtist;
            
    $this->aData['title'] = $sTitle;
            return;
        }
        
        public function 
    getArtist()
        {
            return ( isset(
    $this->aData['artist']) ) ? $this->aData['artist'] : null ;
        }
        
        public function 
    getTitle()
        {
            return ( isset(
    $this->aData['title']) ) ? $this->aData['title'] : null ;
        }

    ChartEntry
    PHP Code:
    class ChartEntry
    {
        private 
    $aData = array();
        
        public function 
    __constructSong $oSong$iPosition$iPositionLastWeek )
        {
            
    $this->aData['artist'] = $oSong->getArtist();
            
    $this->aData['title'] = $oSong->getTitle();
            
    $this->aData['position'] = $iPosition;
            
    $this->aData['positionLastWeek'] = $iPositionLastWeek;
        }
        
        public function 
    getArtist()
        {
            return 
    $this->aData['artist'];
        }
        
        public function 
    getTitle()
        {
            return 
    $this->aData['title'];
        }
        
        public function 
    getPosition()
        {
            return 
    $this->aData['position'];
        }
        
        public function 
    getPositionLastWeek()
        {
            return 
    $this->aData['positionLastWeek'];
        }

    MusicChart
    PHP Code:
    class MusicChart
    {
        private 
    $aChart = array();
        
        public function 
    addEntryChartEntry $oEntry )
        {
            
    $this->aChart$oEntry->getPosition() ] = $oEntry;
        }
        
        public function 
    getEntry$iPosition )
        {
            return ( isset(
    $this->aChart$iPosition ]) ) ? $this->aChart$iPosition ] : null ;
        }

    Usage
    PHP Code:
    //--> Creation
    $oSinglesChart = new MusicChart();
    $oSinglesChart->addEntry( new ChartEntry( new Song'Elvis Presley''Blue Suede Shoes' ), 1) );

    //--> Output
    echo sprintf("At this weeks number %s, it's %s with %s!"
        
    $oSinglesChart->getEntry(1)->getPosition(),
        
    $oSinglesChart->getEntry(1)->getArtist(),
        
    $oSinglesChart->getEntry(1)->getTitle()
    ); 
    Here are some of my concerns:-

    • I created the Song object separate from a ChartEntry because I figured a Song doesn't need to know what were using it for, so it wouldn't hold it current chart position for example. I could also quite easily reuse the Song object to search for a matching Youtube video..... does that sound correct?



    • The interaction between ChartEntry and Song seems a little, well, lame. It merely adds information relating to position then wraps the Song object methods, is this the way composition should work?



    • By passing an object to another object I am adding dependency, what would happen if I renamed the methods in Song? How would I handle this because ChartEntry would then go pop! Or is this why I added ChartEntry as an intermediate layer between Song and MusicChart, so code changes wouldn't cascade to other objects?



    • When building the MusicChart object, It just seems a little messy to me, what about you?



    • Each object to me, just seems to be a container for data, or a glorified Array. It's probably just because this implementation is so basic, isn't it?


    I feel I'm starting to understand elements of OO, and I'm assuming I'm going the right way about it, I just fear I'm doing it blindly...

    Your insight would be most appreciated!

    Anthony.
    Last edited by AnthonySterling; Nov 14, 2008 at 06:47.
    @AnthonySterling: I'm a PHP developer, a consultant for oopnorth.com and the organiser of @phpne, a PHP User Group covering the North-East of England.

  2. #2
    SitePoint Addict
    Join Date
    Feb 2007
    Posts
    251
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I'd get rid of the ChartEntry class. Just store the entry info directly in the MusicChart class. And you're right that these classes seem like glorified arrays, but you'll undoubtedly move more behavior into them as time goes on. For starters, you can get the report building in there.

    PHP Code:
    $chart = new SongChart('Nursing Home Top 40');
    $chart->addSong(new Song('Elvis Presley''Blue Suede Shoes'), 1);
    $chart->addSong(new Song('Elvis Costello''Angels Wanna Wear My Red Shoes'), 2);
    $chart->addSong(new Song('Hank Williams''Wearing Out Your Walking Shoes'), 3);
    echo 
    $chart->getReportForPosition(1); 
    If things get more complicated in terms of reporting, you could use the visitor pattern instead. Maybe overkill, but it's a common approach anyways.

    PHP Code:
    class Song {
        
    //...
        
    function toArray() {
            return 
    $this->data;
        }

    class 
    SongChart {
        
    //...
        
    function getSongAtPosition($i) {
            
    //...
        
    }
        function 
    accept($visitor) {
            
    $visitor->visit($this);
        }

    class 
    TopSongReport {
        function 
    __construct($numSongs) {
            
    $this->numSongs $numSongs;
        }
        function 
    visit(SongChart $chart) {
            
    $this->chart $chart;
        }
        function 
    __toString() {
            
    $lines = array();
            for (
    $i 1$i <= $this->numSongs$i++) {
                
    extract($this->chart->getSongAtPosition($i)->toArray());
                
    $lines[] = "At this weeks number $i, it's $artist with $title!";
            }
            return 
    '<ul><li>' implode('</li><li>'$lines) . '</ul>';
        }
    }

    $chart->accept($report = new TopSongReport(2));
    echo 
    $report

  3. #3
    SitePoint Guru
    Join Date
    Jan 2005
    Location
    heaven
    Posts
    953
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    A couple things, though these are person preferences more than anything else.

    Use protected for class' member variables and functions rather than private. The protected label will facilitate future inheritance chains.

    By passing an object to another object I am adding dependency, what would happen if I renamed the methods in Song? How would I handle this because ChartEntry would then go pop! Or is this why I added ChartEntry as an intermediate layer between Song and MusicChart, so code changes wouldn't cascade to other objects?
    Program to interfaces rather than to a class whenever permissible. On small projects this over complicates things, but as a project grows, it's an effective time saving tool.

    Each object to me, just seems to be a container for data, or a glorified Array. It's probably just because this implementation is so basic, isn't it?
    Use variables rather than arrays. It's cleaner and promotes code readability.

    Objects and arrays share similarities and many times both can be used interchangeably. The benefit of an object over an array is that you have the ability to apply methods which may or may not be beneficial. I was reared in C++, so I tend to use classes as I would a Struct in C++.

  4. #4
    SitePoint Wizard bronze trophy
    Join Date
    Jul 2006
    Location
    Augusta, Georgia, United States
    Posts
    4,187
    Mentioned
    17 Post(s)
    Tagged
    4 Thread(s)
    In this case I think it would be best to use properties rather then a array of values. It doesn't seem as if your adding properties at run time so there isn't much of a point to using a array rather then defining distinct properties of the class.

    imaginethis wrote
    Use protected for class' member variables and functions rather than private. The protected label will facilitate future inheritance chains.
    Perhaps extending classes should not inherit direct access to this property.

    This essentially describes the same data structure, but is not a glorified array which makes it more concrete and easier to read.

    Code:
    class Song {
    
    	private $_artist;
    	private $_title;
    	
    	public function __construct($pArtist,$pTitle) {
    		$this->_artist = $pArtist;
    		$this->_title = $pTitle;
    	}
    	
    	public function getArtist() {
    		return $this->_artist;
    	}
    	
    	public function getTitle() {
    		return $this->_title;
    	}
    
    }
    Furthermore, in your example it seems redundant to be checking for a array key which you know will be there.

    Out of curiosity – will there be one chart or several? For example can a individual song have positions on several charts?

  5. #5
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by oddz View Post
    Perhaps extending classes should not inherit direct access to this property.
    One view on this (to which I ascribe), is that if you have this requirement, then it's a very sure sign that you are really dealing with two separate entities, clumped together in the same class. Split them into two classes and replace the inheritance with composition.

  6. #6
    Twitter: @AnthonySterling silver trophy AnthonySterling's Avatar
    Join Date
    Apr 2008
    Location
    North-East, UK.
    Posts
    6,111
    Mentioned
    3 Post(s)
    Tagged
    0 Thread(s)
    Thanks for all the feedback guys, I still have a few concerns but I'm going to take on board your suggestions and refactor most of the code.

    I'm finding it quite strange that, unusually for me, I'm enjoying the theory behind the OOP structure more than coding.
    @AnthonySterling: I'm a PHP developer, a consultant for oopnorth.com and the organiser of @phpne, a PHP User Group covering the North-East of England.


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •