Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have a page Object, which has Paragraph and Image object Collections

And each paragraph has only image_id(s) that are assigned to a paragraph. All other information about image is stored in Page->Image

Now from the paragraph object I want to access the image information where paragraph has only image_id and all other information about the image is in Page->Image object.

What will be the better way to access this info?

Should I pass the page object in every paragraph constructor or something else?

I also can't change the class structure, as it is written by someone else.

share|improve this question
    
Even if the class has been written by someone else, you can always use the decorator pattern to extend/change its behavior without breaking any OO rules. –  rvcoutinho Jan 11 '13 at 16:45
add comment

3 Answers

up vote 1 down vote accepted

KISS is always the best approach:

foreach($page->paragraphs as $paragraph)
    foreach($paragraph->image_ids as $image_id)
        $image = $page->images[$image_id]

But if you wanted something a little more special a delegate can assist in providing the "missing" functionality you require and for which php 5.3 has made things super easy by means of a little magic.

<?php

class PageDelegate
{
    private $page;

    public function construct($page)
    {
        $this->page = $page;
    }

    /* The magic delegating methods __call, __set and __get */
    public  function __call($name, $args)
    {
        if (method_exists($this->page, $name))
            return call_user_func_array(array($this->page, $name), $args);
        // you may require some conversion to valid method name
        // like converting - to _ etc. this should be kept in mind.
        if (array_has_key($this->images, $name))
            return $this->images[$name]; // note how this becomes $this->page->images[$name]
        throw BadMethodCallException("Method $name does not exist");
    }
    public function __get($name)
    {
        if (property_exists($this->page, $name))
            return $this->page->{$name};
        throw InvalidArgumentException("Property $name does not exist");
    }
    public function __set($name, $value)
    {
        if (property_exists($this->page, $name))
            $this->page->{$name} = $value;
        else
            throw InvalidArgumentException("Property $name does not exist");
    }

    // And perhaps something to help out with common tasks
    public function images_in_paragraph($paragraph) 
    {
        $images = array();
        foreach($paragraph->image_ids as $image_id)
            // Note how this becomes $this-page->images[$image_id];
            $images[] = $this->{$image_id}();
        // return the collection of image objects in the paragraph
        return $images;
    }
}

Using just the 3 magic methods __call, __set and __get we are effectively delegating to all instance members of Page and you can now call for images using their image id as a method. If there are any Page functionality you want to overwrite, simply adding the function would see it being called instead of our magic __call method to do your bidding. Now it doesn't matter that you can't change their source, you can still do what you please. =)

The implementation would then look something like this perhaps

$page = new PageDelegate($page);
foreach($page->paragraphs as $paragraph)
    foreach($page->images_in_paragraph($paragraph) as $image)
        echo "<img title=\"$image->title\" src=\"$image->src\">";

nJoy!

share|improve this answer
    
disclaimer: I just typed this- out here so I can't be sure if there might be compilation problems. Looks about right, yes? Do shout and see it rectified asap. –  nickl- Jan 14 '13 at 4:52
    
That breaks law of Demeter and therefore class encapsulation. –  jgauffin Jan 14 '13 at 7:05
    
Yes of course the delegator knows more about the delegates in its possession and employs exactly what it requires to accomplish its task. Exposing a new interface while still encapsulating the data of a Page while hiding the underlying complexities. No limit is imposed on the number, depth, interfaces used by a delegator, none that I am aware of. What you fail to bring into account is the simple restriction I also can't change the class structure without which the corrections could be made in those interfaces instead. @jgauffin I would like to see your solution without infringing on LoD-F. –  nickl- Jan 15 '13 at 1:15
add comment

I think rvcoutinho's comment is in line with the correct answer. Either the page class or a 3rd class (decorator/mediator) should be responsible for providing the image to the paragraph. Any images that the paragraph requires should be passed to the paragraph.

If you don't like either of those options, I would think that passing the Image Collection to the paragraph is far preferable than passing the page class. After all, if you just think about the problem conceptually, pages contain paragraphs. Paragraphs don't contain or know about pages.

share|improve this answer
    
The reason I consciously chose to avoid a decorator is to loosely couple any dependency to this foreign system which we do not have control over. We are effectively creating our own Page interface independent of the data source of its parts. Since no special need has been identified as yet, we simply mirror the delegate. Any changes in their structure can be made transparent by the delegator alone. The decorator would itself suffer this consequence as well as every part of the system still exposed to its parent. –  nickl- Jan 15 '13 at 1:44
    
The mediator, is perfect for loosely coupling interactions between disparate data sources and this is where we need to make the distinction. As a behavioural pattern I would implore a mediator for reaching conglomerate conclusions. The requirement however is simply to expose the information. There is only one image per image_id at one location, hardly something that requires mediation, would you think? –  nickl- Jan 15 '13 at 2:18
    
Write the Page Class (your application) the way it makes the most sense in a well designed system. If that means putting a layer to turn a 3rd party system (that isn't particularly well designed for your application) then so be it. You will be stuck with your design forever. The 3rd party API very well may vanish in the future and you will still be stuck with your application. If your application is convoluted because you wanted to use a 3rd party library/app that has now vanished, you are still stuck with the convolution that no longer needs to be there. If you layered it then issue resolved. –  Dunk Feb 4 '13 at 15:53
add comment

The paragraph object need to have some mechanism to know where it's image data comes from.

Assuming that a paragraph can't exist in isolation but only as part of a page, there's no harm in adding the owning page as a constructor parameter for each paragraph in order to supply this.

share|improve this answer
2  
I disagree. There is great harm anytime you create a circular dependency, as would be the case of passing page to the paragraph. Now the paragraph needs to know about pages and pages needs to know about paragraphs. A change to either could break both. –  Dunk Jan 14 '13 at 15:10
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.