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

use class_alias instead of DynamicParent class and autoload with psr-0 #15

Closed
raffaelj opened this issue Mar 26, 2023 · 5 comments
Closed
Labels
Enhancement Issues that will iterate on existing functionality.

Comments

@raffaelj
Copy link
Contributor

I found a cleaner solution to dynamically load Parsedown or ParsedownExtra as parent: class_alias.

Here is my implementation in action:
https://codeberg.org/raffaelj/parsedown-tasks/src/branch/main/ParsedownTasks.php#L6-L14

This removes one problem of multiple classes named DynamicParent in the global scope, when trying to mix multiple Parsedown addons, that use that same technique.

If you want to implement it, make sure to check for is_callable('parent::__construct') in your constructor to fix the missing constructor in Parsedown.

And while I'm here - instead of opening another issue - it's better to autoload with psr-0 instead of with files. The difference is, that autoloading with files always includes the file, when require(__DIR__.'/vendor/autoload.php'); is called. Autoloading with psr-0 only includes the file when the class is in use, e. g. $test = new Test();. This is a huge performance improvement when dealing with many classes.

Thanks again for your work and have a nice day,
Raffael

@BenjaminHoegh
Copy link
Owner

Now I wonder how I did not stumble upon class_alias before, it is amazing :D

I will implement your changes :)

@BenjaminHoegh BenjaminHoegh added Enhancement Issues that will iterate on existing functionality. In Progress In Progress labels Aug 12, 2023
@raffaelj
Copy link
Contributor Author

Now I wonder how I did not stumble upon class_alias before, it is amazing :D

I reacted the same way. When I followed an outdated example on StackOverflow for working with SMTP in WordPress, I found this simple function, that changes everything. WordPress uses this technique to throw a deprection notice for a renamed vendor namespace.

It feels like a hack - and it probably is one. So don't over-use it, when there are better alternatives. But extending Parsedown 1.x, feels like the perfect match.

@BenjaminHoegh BenjaminHoegh removed the In Progress In Progress label Aug 13, 2023
@raffaelj
Copy link
Contributor Author

You missed one point:

If you want to implement it, make sure to check for is_callable('parent::__construct') in your constructor to fix the missing constructor in Parsedown.

@raffaelj
Copy link
Contributor Author

And you can replace lines like

$Block = ParsedownTocParentAlias::blockHeader($Line);

with

$Block = parent::blockHeader($Line);

@BenjaminHoegh
Copy link
Owner

wait why did I not thing about that... that make so much sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Issues that will iterate on existing functionality.
Projects
None yet
Development

No branches or pull requests

2 participants