prismicio-community / php-kit

Community maintained development kit for Prismic and the PHP language
https://prismic.io
Other
109 stars 83 forks source link

Change private declarations to protected #25

Closed tremby closed 10 years ago

tremby commented 10 years ago

Allowing extension of classes

dohzya commented 10 years ago

I'm not confortable with allowing subclasses to access to superclass instance variables… I think subclasses should use their superclass's accessors instead. I prefer to mask the fact that a particular variable is used (and to limit the places where this variable is used) and to provide a stable API (on which other elements—included subclasses) can rely.

Does something in particular interfere with your work?

tremby commented 10 years ago

Yes. What I'm doing (with my fork which has this patch applied) is extending the \Prismic\Document class to provide (lots) more methods -- I'm doing what I was asking for in #24. Here's my extended class.

<?php

class PrismicDocument extends \Prismic\Document {

    /**
     * Constructor, takes a \Prismic\Document and turns it into this class
     */
    public function __construct(\Prismic\Document $document) {
        $this->id = $document->getId();
        $this->type = $document->getType();
        $this->href = $document->getHref();
        $this->tags = $document->getTags();
        $this->slugs = $document->getSlugs();
        $this->fragments = $document->getFragments();
    }

    /**
     * Extended methods to automatically provide LinkResolver
     */
    public function getHtml($field, $linkResolver = null) {
        if (is_null($linkResolver)) {
            $linkResolver = new LinkResolver;
        }
        return parent::getHtml($field, $linkResolver);
    }
    public function asHtml($linkResolver = null) {
        if (is_null($linkResolver)) {
            $linkResolver = new LinkResolver;
        }
        return parent::asHtml($linkResolver);
    }

    /**
     * Methods to get this_document_type.field_name in various formats
     */
    public function getMy($field) {
        return $this->get($this->getType() . '.' . $field);
    }
    public function getAllMy($field) {
        return $this->getAll($this->getType() . '.' . $field);
    }
    public function getMyText($field) {
        return $this->getText($this->getType() . '.' . $field);
    }
    public function getMyNumber($field) {
        return $this->getNumber($this->getType() . '.' . $field);
    }
    public function getMyBoolean($field) {
        return $this->getBoolean($this->getType() . '.' . $field);
    }
    public function getMyDate($field) {
        return $this->getDate($this->getType() . '.' . $field);
    }
    public function getMyHtml($field) {
        return $this->getHtml($this->getType() . '.' . $field);
    }
    public function getMyImage($field) {
        return $this->getImage($this->getType() . '.' . $field);
    }
    public function getAllMyImages($field) {
        return $this->getAllImages($this->getType() . '.' . $field);
    }
    public function getMyImageView($field, $view = null) {
        return $this->getImageView($this->getType() . '.' . $field, $view);
    }
    public function getAllMyImageViews($field, $view = null) {
        return $this->getAllImageViews($this->getType() . '.' . $field, $view);
    }
    public function getMyStructuredText($field) {
        return $this->getStructuredText($this->getType() . '.' . $field);
    }
    public function getMyGroup($field) {
        return $this->getGroup($this->getType() . '.' . $field);
    }

}

On a call to my method getMyText I get array_key_exists() expects parameter 2 to be array, null given thrown at https://github.com/prismicio/php-kit/blob/master/src/Prismic/Document.php#L91

Stack trace goes

  1. https://github.com/prismicio/php-kit/blob/master/src/Prismic/Document.php#L91
  2. https://github.com/prismicio/php-kit/blob/master/src/Prismic/Document.php#L130
  3. (my class's getMyText method)

If there's another way to work around that I'm all ears.

I should note that after I call the API to get a list of results I am looping through them and replacing each \Prismic\Document with a new PrismicDocument($document).

dohzya commented 10 years ago

It seems the only problem is that you shadow the Document's instance variables instead of setting them. Can you use parent::__contruct instead of trying to set them by hand?

tremby commented 10 years ago

Oh boy, that was stupid of me. Thanks!