render non markdown docs with github api still

[mattstauffer]

Seems like it actually works?!

[mattstauffer]

is there a cleaner way to decorate $this->content conditionally?

[mattstauffer]

help?

[adamwathan]

I'd rename the other one to loadJsonFixture and make this one just loadFixture.

[mattstauffer]

Tag @JacobBennett

[JacobBennett]

Looks like a fun challenge :) I'll take a look tonight. 

[adamwathan]

Use polymorphism, something like this...

<?php

$gistFactoryMap = [
    'Markdown' => function ($gist, $comments) {
        // Could also inject the Cache and ContentParser here and get rid of evil
        // facade usage.
        return new MarkdownGistlog(Gistlog::fromGitHub($gist, $comments));
    }
];

class GistlogRepository
{
    private $gistClient;
    private $gistFactoryMap;

    public function __construct(GistClient $gistClient, $gistFactoryMap)
    {
        $this->gistClient = $gistClient;
        $this->gistFactoryMap = $gistFactoryMap;
    }

    public function findById($id)
    {
        $gist = $this->gistClient->getGist($id);
        $comments = $this->gistClient->getGistComments($id);

        return $this->getGistBuilder($gist['files'][0]['type'])->__invoke($gist, $comments);
    }

    private function getGistBuilder($language)
    {
        // Default to the old one
        return array_get($this->gistFactoryMap, $language, function () {
            return function ($gist, $comments) {
                return Gistlog::fromGitHub($gist, $comments);
            };
        });
    }

    // ...and all those other methods, etc.
}

class MarkdownGistlog
{
    private $gistlog;

    public function __construct($gistlog)
    {
        $this->gistlog = $gistlog;
    }

    public function render()
    {
        if ($this->updatedAt == Cache::get('markdown.updated_at.' . $this->id)) {
            return Cache::get('markdown.' . $this->id);
        }
        $markdown = ContentParser::transform($this->content);
        Cache::forever('markdown.' . $this->id, $markdown);
        Cache::forever('markdown.updated_at.' . $this->id, $this->updatedAt);
        return $markdown;
    }

    public function __get($key)
    {
        return $this->gistlog->{$key};
    }
}

class Gistlog
{
    //... all the old stuff +

    public function render()
    {
        return "<pre><code>" . htmlspecialchars($this->content) . "\n</code></pre>";
    }
}

Would make it really easy to add other types down the line if other languages need to be rendered differently.

[JacobBennett]

Yeah @adamwathan totally what I was thinking... took the words right out of my mouth... you just got to it before I did :) (this was sarcasm btw).

Also this has been up in my browser since you said "Replace Conditional with Polymorphism" at Laracon. Great use case!

After looking through this block of code you've written, so many cool things I wouldn't have thought to use. Mostly the use of closures: in the factory map and as a default for array_get.

I like the use of the decorator to change the render method. I would have reached for inheritance first. Is there a benefit to using the decorator pattern over inheritance in this instance or a rule of thumb to use when determining which is appropriate? @adamwathan @mattstauffer

[JacobBennett]

@mattstauffer since you are working on gistlog today... 👍 any chance you still want this one taken care of?

[mattstauffer]

@JacobBennett hahaha, I'm literally looking at this right now.

[mattstauffer]

@JacobBennett is gonna take this one for HacktoberFest 2017. For some reason i can't assign him. ¯(°_o)/¯

[mattstauffer]

... For some reason GitHub won't let me change the target branch! This is not really closed.