Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: factory methods or classes to avoid "boilerplate" code #62

Open
holtkamp opened this issue Aug 18, 2018 · 2 comments
Open

Comments

@holtkamp
Copy link

Is your feature request related to a problem? Please describe.
reduce need for "boilerplate" functions

Describe the solution you'd like
I found myself implementing static factory functions to easily assemble a SitemapIndex or UrlSet:

SitemapIndex::getInstanceForSitemaps($sitemaps): static
UrlSet::getInstanceForUrls($urls): static

Describe alternatives you've considered
Instead of using static factory functions, we might use dedicated factory classes:

SitemapIndexFactory::getInstance($sitemaps = null): Sitemap
UrlSetFactory::getInstance($urls = null): UrlSet

Additional context
Implementation is quite trivial, I currently have:

class SitemapIndex extends \Thepixeldeveloper\Sitemap\SitemapIndex
{
    public static function getInstanceForSitemaps($sitemaps) : self
    {
        $sitemapIndex = new static();
        foreach ($sitemaps as $sitemap) {
            $sitemapIndex->addSitemap($sitemap);
        }

        return $sitemapIndex;
    }
}
class UrlSet extends \Thepixeldeveloper\Sitemap\UrlSet
{
    /**
     * @param OutputInterface[] $urls
     *
     * @return static
     */
    public static function getInstanceForUrls($urls) : self
    {
        $urlSet = new static();
        foreach ($urls as $url) {
            $urlSet->addUrl($url);
        }

        return $urlSet;
    }
}

I can provide a PR, would it be considered? Any suggestions / preference?

@ThePixelDeveloper
Copy link
Owner

ThePixelDeveloper commented Aug 23, 2018

I wouldn't mind having a factory class. The only preference I have would be naming the factory method something along the lines of "fromUrls", so UrlSetFactory::fromUrls();

Apart from that, go for it.

Thanks for the contribution @holtkamp .

@holtkamp
Copy link
Author

@ThePixelDeveloper thanks for the response, then I will see when I got time to come up with a PR using FactoryClasses 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants