NOTE: Parent Task: {T15190}
We call so much time `strlen()`.
Why? One of the reasons is that it's used to check `PhabricatorEnv::getEnvConfig()` - whenever it returns an empty value or not.
Why? Because the method `is_empty()` is fondamentally broken - it thinks that the string `"0"` is empty, for example. So we use `strlen()`.
Why? Because the method `PhabricatorEnv::getEnvConfig()` returns a mixed value. The problem is, most of our checks are something like this:
```lang=php
public function newDefaultEmailAddress() {
$test = PhabricatorEnv::getEnvConfig('test');
if (strlen($test)) {
```
Note that checking `strlen()` had sense to both assume that:
- it was a string
- it is not an empty string
Note that, we use `strlen()` to just check if it's zero, but `strlen()` is slower than `is_empty()` - since `strlen()` counts every single character, while `is_empty()` just tells whether it is empty or not.
== Why a new method could be useful ==
Note that, without changing anything else, the natural change should be this one now, that is literally the fix:
```lang=diff
public function newDefaultEmailAddress() {
$test = PhabricatorEnv::getEnvConfig('test');
- if (strlen($test)) {
+ if ($test != null && strlen($test)) {
return $test;
```
Why? Why do we count for the length of the string? We just need to verify if it's empty, after all.
Probably, we do that, since this is the natural equivalent, but it's too much long:
```lang=diff
public function newDefaultEmailAddress() {
$test = PhabricatorEnv::getEnvConfig('test');
- if (strlen($test)) {
+ if ($test !== null && is_string( $text) && $text !== '') {
return $test;
```
== Introducing getEnvConfigString() ==
So, we could introduce this simple wrapper method:
```lang=php
public static function getEnvConfigString($key) {
return (string) self::getEnvConfig($key);
}
```
The above method is very efficient since:
- in the base case, any long string is just returned as-is
- the NULL value is just converted as '' (very quick operation)
- any other weird nonsense type is just returned as an empty string (very quick operation)
So, introducing `getEnvConfigString()`, every use can eliminate two tests, since they can assume that the return value is a string, and it's never NULL. So the return value can be just an empty string, or a longer string.
So, this would be the average usage:
```lang=diff
public function newDefaultEmailAddress() {
- $test = PhabricatorEnv::getEnvConfig('test');
+ $test = PhabricatorEnv::getEnvConfigString('test');
if ($test !== '') {
return $test;
```
This would be:
- so much readable
- so much efficient (we don't count anymore the length of the string to just check whenever it's empty - we save CPU cycles - we save energy - we save the environment - Greta Thunberg is happier)