Refactoring function to collect constants with a certain prefix

Question

I have a function that aims to locate and group in a matrix the constants that are declared corresponding to a certain prefix:

Function:

function get_constantsByPrefix($prefix) {

  foreach (get_defined_constants() as $key=>$value) {
    if (substr($key,0,strlen($prefix))==$prefix) {
      $dump[$key] = $value;
    }
  }

  if (empty($dump)) {
    return "Atenção: Não foram localizadas constantes com o prefixo '".$prefix."'";
  } else {
    return $dump;
  }
}

Example of use:

print_r(get_constantsByPrefix('CON_WEBSITE_'));

Exit:

Array ( [CON_WEBSITE_01] => John [CON_WEBSITE_02] => Doe ) 

This function makes use of the PHP function get_defined_constants() to verify all the declared constants and from this result, check which ones correspond to the given prefix.
If found, add to an array, returning at the end an array with the localized constants or a warning message if none have been located.

Question:

This function can be optimised by allowing code reduction and/or efficiency gain in its execution?

Answer 1

I know I’m not exactly answering the question as I seem to specifically want a performance improvement, but I would like to make some suggestions to improve the clarity of the code.

As it stands, one can say that the function is doing "too many things": it verifies that there are constants And creates a message for the user -- are two different subjects, it may not be interesting to be connected.

How the function name is get_constantsByPrefix(), it is reasonable to assume that it will return a list of constants (after all, it is get_constants() and not get_constants_or_error_message()). So it is better to return an empty list when no constant is found for the prefix.

Therefore, I suggest removing the if/Else from the end and returning the $dump straightforward. =)

That way, when you write the client code (that is, the code that calls the function) you don’t have to remember to check the type of the returned variable to see if there are constants. And if you want to show a message to the user just check if the list is empty. =)

Answer 2

Taking advantage of the @Lias hook, if the non-existence of constants is something serious in your application, a more consistent alternative to error message return in functions is the use of exceptions, thus:

if (empty($dump)) 
    throw new Exception("Atenção: Não foram localizadas constantes com o prefixo '".$prefix."'");

However, if it is just something informative, log the message and return an empty list.

Furthermore, consider also the principle of single responsibility and, depending on the purpose of this message, place this if in the "client" code that calls the method. See an example:

define('CON_WEBSITE_01', 'John');
(...)
$constants = get_constantsByPrefix('CON_WEBSITE_');
print_r( empty($constants) ? "Nada encontrado" :  $constants );

---

Performance of the function get_constantsByPrefix(), depending on the use case, we may consider the following factors:

Constants can be defined in the "middle" of the main system execution or they are usually defined on startup, for example, in addition of classes at the beginning of the script?

Depending on the case, it would be worth storing the return map for later use, rather than always iterating over all constants.

Let’s scribble an example using a class:

class Constants {

    private $constant_map = null;

    public static function listByPrefix($prefix) {

        if ($this->constant_map == null) {
            $this->constant_map = array();
            foreach (get_defined_constants() as $key=>$value) {
                if (substr($key,0,strlen($prefix))==$prefix) {
                    $this->constant_map[$key] = $value;
                }
            }
        }

        if (empty($this->constant_map)) {
            throw new Exception("Atenção: Não foram localizadas constantes com o prefixo '".$prefix."'");
        } else {
            return $this->constant_map;
        }

    }

}

Example of use:

define('CON_WEBSITE_01', 'John');
Constants::listByPrefix('CON_WEBSITE_');

You have control over the definition of constants?

In this case, you could encapsulate their creation with a function that already stores them on the map.

Another sketch:

class Constants {

    private $constant_map = array();

    public static function define($key, $value) {
        
        define($key);
        $this->constant_map[$key] = $value;
        
    }

    public static function listByPrefix($prefix) {

        if (empty($this->constant_map)) {
            throw new Exception("Atenção: Não foram localizadas constantes com o prefixo '".$prefix."'");
        } else {
            return $this->constant_map;
        }

    }

}

Example of use:

Constants::define('CON_WEBSITE_01', 'John');
Constants::listByPrefix('CON_WEBSITE_');

Answer 3

Suggestion for code reduction (already removing the warning message suggested by @Lias):

function get_constantsByPrefix($prefix) {
  foreach (get_defined_constants() as $key=>$value) {
    if($prefix == "" || strpos($key, $prefix) === 0) $dump[$key] = $value;
  }
 return $dump;
}

---

PS: I didn’t make one benchmarking performance.

PS2: Also set the consistency to return all constants if $prefix come empty (if you want to use the same function for all situations).