cypht-org / cypht

Cypht: Lightweight Open Source webmail aggregator [PHP, JS]
http://cypht.org
GNU Lesser General Public License v2.1
949 stars 146 forks source link

Rethinking Hm_Functions #1048

Open TheDigitalOrchard opened 1 month ago

TheDigitalOrchard commented 1 month ago

🗣 Suggestion

I'll start off by saying that my Unit Testing experience is very limited. As the sole developer for my projects, I haven't had the need to do a lot of package-wide unit testing, although I'm sure I'd get a lot of value if I finally did incorprate this best practice.

In lib/framework.php, a class called Hm_Functions is defined with a lot of methods that merely wrap around built-in functions, such as function_exists() and class_exists(). Calling these results in two function calls, even though nothing of value is added by this.

The explanation for this class is:

/**
     * Used to override built in functions that break unit tests
     * @package framework
     * @subpackage setup
     */

Can someone enlighten me as to how unit tests are broken by built-in functions? What is an example?

How is calling Hm_Functions::function_exists() better than just calling the built-in function_exists()?

Let's start there. Help me understand this better. If there is a solid explanation for using this wrapping class, then I'll drop my suggestion for rethinking it.

(Yes, I'm a micro-optimization nut, so I'm always triggered by unnecessary function calls, pun intended)

Thanks.

marclaporte commented 1 month ago

@josaphatim Please advise

josaphatim commented 3 weeks ago

@TheDigitalOrchard I tried to replace calls of Hm_Functions::function_exists and Hm_Functions::class_exists by native functions: there was a total of 3 tests failed. It can be more if I replaced all methods in Hm_Functions. I will investigate more but of course Hm_Functions prevents to break unit test