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

Refactor to make it easier to add new formats #50

Open
aaronpk opened this issue Dec 27, 2017 · 2 comments
Open

Refactor to make it easier to add new formats #50

aaronpk opened this issue Dec 27, 2017 · 2 comments

Comments

@aaronpk
Copy link
Owner

aaronpk commented Dec 27, 2017

Adding a new format should require only adding a new Format class and declaring it somewhere. Currently it requires changing a few files because of the way the URL matching works.

@Zegnat
Copy link
Contributor

Zegnat commented Dec 28, 2017

Brain storm following.

I think it makes sense to say that the XRay library depends on these Format classes, so passing them at construct time feels right:

// Only let XRay parse Hackernews and XKCD:
$xray = new \p3k\Xray([
    new \p3k\XRay\Formats\Hackernews,
    new \p3k\XRay\Formats\XKCD,
]);
// Should result in an empty output as no parsers work:
$output = $xray->parse('https://example.com');

If no list of formats is provided, it could use some sort of default internal list. If the default list is exposed publicly as a static, users who only want to add a format could retrieve it prior to instantiating XRay:

$formats = array_merge(
    \p3k\Xray::default_formats(),
    [ new \app\XRayFormats\Example ]
);
// Use all default formats plus my own:
$xray = new \p3k\Xray($formats);

This also means there is no need for the formats to be in any specific folder. When XRay is included as a dependency through composer, people will not have to dig through the vendor directory looking for formats.

Another nice thing is that, currently, XRay does some array juggling to find credentials for different APIs. If formats are actual objects they can also have constructors requiring this information. XRay does not need to know about which formats require what authentication, it is up to the formats themselves:

// Parse YouTube through the API:
$xray = new \p3k\Xray([
    new \app\XRayFormats\YouTube($config['apikey'])
]);

@aaronpk
Copy link
Owner Author

aaronpk commented May 18, 2020

I made some progress on this in this commit 989d42a, just in terms of now more of the logic is funneled through the same place.

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