Function refactoring to collect constants with a specific prefix

4

I have a working function that aims to find 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_'));

Output:

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

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

Question:

Is it possible to optimize this function by allowing code reduction and / or efficiency gain in execution?

    
asked by anonymous 18.12.2013 / 22:56

3 answers

4

I know I'm not exactly answering the question because it seems to specifically want performance improvement, but I'd like to make some suggestions to improve the clarity of the code.

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

Because 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's best to return an empty list when you do not find any constants for the prefix.

So I suggest removing the if / else from the end and returning the $dump direct. =)

That way, when you write the client code (that is, the code that calls the function) you do not 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. =)

    
19.12.2013 / 02:42
0

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

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

PS: I did not benchmarking performance.

PS2: Consistency is also given to return all constants, if $prefix is empty (if you want to use the same function for all situations).

    
19.12.2013 / 12:45
0

Taking advantage of the @elias hook, if non-existence of constants is somewhat serious in your application, a more consistent alternative to returning error messages in functions is to use exceptions, like this:

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

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

Also, consider the principle of single responsibility and, depending on the use of this message, put this if into the "client" code, which calls the method. Here's an example:

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

Regarding the performance of the get_constantsByPrefix() function, depending on the use case, we can consider the following factors:

Can constants be defined in the middle of the main system execution, or are they usually set at startup, for example, by adding 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 usage:

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

Do you have control over setting constants?

In this case, you could encapsulate creating them 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 usage:

Constants::define('CON_WEBSITE_01', 'John');
Constants::listByPrefix('CON_WEBSITE_');
    
19.12.2013 / 11:31