PHP Classes

There are some problems with your code.

Recommend this page to a friend!

      Constant Array 2  >  All threads  >  There are some problems with your code.  >  (Un) Subscribe thread alerts  
Subject:There are some problems with your code.
Summary:Package rating comment
Messages:3
Author:Artur Graniszewski
Date:2011-02-09 10:41:14
Update:2011-02-09 21:55:46
 

Artur Graniszewski rated this package as follows:

Utility: Not sure
Consistency: Sufficient

  1. There are some problems with your code.   Reply   Report abuse  
Picture of Artur Graniszewski Artur Graniszewski - 2011-02-09 10:41:15
There are some problems with your code.

You really shouldn't use eval() function for security and performance reasons (eval launches second PHP interpreter so it's really slow). To access constants values you may use mixed constant ( string $name ) function (http://pl2.php.net/manual/en/function.constant.php).

What's more you are trying to catch PHP exceptions in blocks of code where exceptions cannot be thrown! For example:

try {
define($constName, var_export($paramArray, true));
} catch (Exception $e) {
throw new Exception('ArrConst Error: Unknown error');
}

In this case define() and var_export() can only trigger PHP errors (warnings, fatals, etc.), they doesn't throw any exceptions.

Another thing to notice is that you should use "break" to bail out from the loops in case of a problem. You use exceptions instead (even thou you catch them later):

try {
for ($i=0; $i <= $keyArrMaxIdx; $i++) {
// make sure the element request key is valid before push
if ($keyArr[$i] != NULL && $keyArr[$i] != ' ' && array_key_exists($keyArr[$i], $constArray)) {
array_push($returnArray, $constArray[$keyArr[$i]]);
} else {
throw new Exception('ArrConst Error: Bad Request Array Element');
}
}
} catch (Exception $e) {
print $e->getMessage();
}

  2. Re: There are some problems with your code.   Reply   Report abuse  
Picture of Ville Walveranta Ville Walveranta - 2011-02-09 18:59:29 - In reply to message 1 from Artur Graniszewski
Hi Artur,

You're right, of course. The code is Sourav Ray's original (you can find the original class here under "Constant Array"), I only fixed an issue that prevented associative array indexes from being retrieved. It works, but as you point out it shoudld be fixed for speed and security reasons.

Ville

  3. Re: There are some problems with your code.   Reply   Report abuse  
Picture of Ville Walveranta Ville Walveranta - 2011-02-09 21:55:46 - In reply to message 1 from Artur Graniszewski
I updated the class with some additional fixes. I eliminated eval per your suggestion; I used serialize instead so that it would not be necessary to call eval on get (using constant() by itself would not work because of the nature of constant content - i.e. arrays).

I also fixed exception handling so that now all thrown exceptions are caught, and eliminated exceptions that could not be caught.

I have a question about breaking out of the for-loop. If I change the exception to break, how do I communicate to the user that an error has occurred? One could of course print a message, but if this class were to be used in a larger setting where the developer wants to bubble all catchable exceptions higher in the control structure - perhaps to a specialized error handler, it would be preferable to be able to throw an exception when a bad key is encountered.