thecodingmachine / safe

All PHP functions, rewritten to throw exceptions instead of returning false
MIT License
2.37k stars 145 forks source link

Library with method calls instead of function calls #366

Open iquito opened 2 years ago

iquito commented 2 years ago

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 https://github.com/thecodingmachine/safe/issues/253 - any "Cannot redeclare functions" errors would not occur with objects and autoloading.

dbrekelmans commented 2 years ago

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.