4

I'm still trying to wrap my head around OOP. All of the following examples work, of course, but is there one (or possibly another) that best exemplifies OOP concepts?

/**
 * For the following examples the Image class queries an image
 * and associated info from a database, which is passed on
 * instantiation via dependency injection.
 */

$image = new Image(new Db());

Example 1:

if ($image->setImageId($id, $size)) {
    header('content-type: ' . $image->content_type);
    header('content-length: ' . $image->length);
    echo $image->getBytes();
}

Example 2:

if ($image_info = $image->getImageInfoByIdAndSize($id, $size)) {
    header('content-type: ' . $image_info->content_type);
    header('content-length: ' . $image_info->length);
    echo $image->getImageBytesByIdAndSize($id, $size);
}

Example 3:

$image->setImageIdAndSize($id, $size);

if ($image_info = $image->getImageInfo()) {
    header('content-type: ' . $image_info->content_type);
    header('content-length: ' . $image_info->length);
    echo $image->getImageBytes();
}
4
  • 9
    "OOP" means a lot of different things to a lot of different people, and like all design, there is rarely a single "best" solutions for object-oriented design. Treat all who claim to have "the best design" with extreme caution.
    – user7043
    Commented Mar 5, 2013 at 18:02
  • When it comes to programming, I interpret best as better. Even the world's fastest computer won't make that claim for very long.
    – JeffO
    Commented Mar 5, 2013 at 19:28
  • @delnan Acording to your opinion, does OOP exist ? Commented Mar 6, 2013 at 13:16
  • 1
    @user61852 It'd be silly to claim it doesn't. It's just that there are many meanings to the term. Both the term and the things the term is used for exist obviously.
    – user7043
    Commented Mar 6, 2013 at 14:14

2 Answers 2

5

I agree with delnan's comment about "OOP", and that there's really no such thing as "proper OOP concepts", but in terms of general coding principles, I think example 2 is a clear winner compared to the others.

Examples 1 & 3 are the same thing with different names. In this case it's not obvious at all that setting the object's ID and size would cause a database fetch and its other properties to be populated. I've debugged code like this and it's hard to track down exactly what's happening with these kinds of side effects hidden all over the place.

Example 2 is a sort of combination of a Data Access Object and a Factory, and these could potentially be separated out, but either way it's more obvious to the reader what is happening.

Edit Here's an even clearer representation:

if ($image = $image_data_access->getImageById($id)) {
    $image->scaleTo($size);
    header('content-type: ' . $image->content_type);
    header('content-length: ' . $image->length);
    echo $image->getBytes();
}
7
  • Yeah, examples 1 & 3 being non-obvious is a real problem
    – Ryan Kinal
    Commented Mar 5, 2013 at 20:23
  • In that case, are you suggesting that I create a "getter" for every property?
    – Isius
    Commented Mar 5, 2013 at 21:19
  • @Isius: Not at all. I'm saying complex behaviors shouldn't be buried in a property setter. Reading a function called setImageIdAndSize, I'd expect to be simply assigning an ID and a size. I might expect scaling of image data because I've changed the size, but I'm definitely not expecting a database call. I added an example. Here you can tell just by reading that we're fetching an image from some data source, then scaling it (as opposed to "resize" which could mean several things), then writing its contents.
    – Misko
    Commented Mar 5, 2013 at 23:06
  • Sorry, I'm a bit confused with "would cause a database fetch and its other properties to be populated". Isn't that the point that you not know about the database? Just that you want to retrieve an image and it's data based on id and size?
    – Isius
    Commented Mar 5, 2013 at 23:25
  • 1
    IMO, it's best to avoid getters/setters unless your intent isn't so much OOP as a data storage structure which most languages have some type/structure for regardless. Encapsulation is what keeps it about objects interacting with other objects vs. just having a pile of functions chaining off each other and stashing data wherever it's convenient to. Commented Mar 5, 2013 at 23:26
1

(my opinion, please do not be offended if you disagree)
None is good.
Method name does not indicate at all that headers are being sent or something is being echoed.
I would prefer something like following.

class Image {

public function __construct($id) {
#load, if fail throw exception
}

private function getHeaders() {
    return array(
        'content-type'   =>  $this->content_type,
        'content-length' => $this->length
    );
}

private function sendHeaders() {
    #calls function sendHeaders defined somewhere far away (usually it is 'app' object which has method like this)
    sendHeaders($this->getHeaders()); 
}

/** Sends image headers and content(bytes) */
public function sendImage() {
    $this->sendHeaders();
    echo $this->getBytes();
}
}

If I am missing 'OOP way' please write in comments as I am also learning, and I would be thankful for directing me in right way :)

1
  • 2
    A couple of things. First, putting data access in a constructor is a bad practice, see here. Second, you have now tightly coupled the Image class to a data source, to the HTTP response, and to some "app" object. Having a class that renders the image to the response stream isn't a bad idea, but it's also not the same role filled by Isius's original class.
    – Misko
    Commented Mar 6, 2013 at 17:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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