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

Library with method calls instead of function calls #366

Open
iquito opened this issue Jun 21, 2022 · 1 comment
Open

Library with method calls instead of function calls #366

iquito opened this issue Jun 21, 2022 · 1 comment
Assignees

Comments

@iquito
Copy link

iquito commented Jun 21, 2022

I think this library is really great, thanks for creating and supporting it! One aspect that I feel could be better is the current use of functions instead of methods. Methods would have the main benefit of autoloading its class, and the library would only be loaded when used. In my setup using preloading all the files from this library come up in the list of most used non-preloaded files. I don't think this has a big performance impact, but it is just noticeable that in PHP where there is no function autoloading but very good class autoloading, a class would be beneficial in terms of performance, usage with composer and the statistics within PHP.

One possibility to use methods is to just have a Safe class with all the current functions implemented as static methods:

// before:
\Safe\file_get_contents('something.txt');

// after:
\Safe\Safe::file_get_contents('something.txt');

// or with a "use Safe\Safe" at the top:
Safe::file_get_contents('something.txt');

This would have a slight performance benefit with clear autoloading mechanics, and I don't see any drawbacks going to static methods compared to functions. Another advantage would be that one only needs one "use" statement for the class instead of multiple "use function" for all the used functions in a file. Of course it would not really make sense to change this within this package (as it would be a crazy BC break), but there could just be a new package thecodingmachine/safe_methods or thecodingmachine/safe_object or whatever you feel is best. If you need any help to adapt the current code to create such a package, I would be willing to help.

Using methods would also be a solution to #253 - any "Cannot redeclare functions" errors would not occur with objects and autoloading.

@Kharhamel Kharhamel self-assigned this Jun 21, 2022
@dbrekelmans
Copy link
Collaborator

Hi @iquito. This seems like a big change and BC-break for a slight performance increase. Do you have any objective numbers of performance improvement? From the comments of #253, it looks like that issue has already been resolved.

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

3 participants