Error Codes for Various Exceptions #132
Conversation
There was a problem hiding this comment.
Commonly used error codes should be defined on Erfurt_Exception and not additionally on the subclasses.
If the error codes are not used resp. there is no check for the error code, just remove it. The only case, where we need unnecessary error codes, as a placeholder as second parameter to provide a third parameter, Erfurt_Exception::DEFAULT_ERROR should be used.
This especially includes:
- if a non 0 value is set, but never checked, replace it with
Erfurt_Exception::DEFAULT_ERROR, - if the last parameter of the Exception constructor is 0 resp.
Erfurt_Exception::DEFAULT_ERROR, remove it.
| @@ -330,7 +345,7 @@ protected function _getNormalizedErrorCode() | |||
| return 1000; | |||
| } | |||
| } else { | |||
| return -1; | |||
| return Erfurt_Exception::DATABASE_ERROR; | |||
There was a problem hiding this comment.
Why do you replace the return value in this case with a constant, but not three lines above?
shinobu
force-pushed
the
feature/Error_Codes
branch
2 times, most recently
from
224a451
to
5a6296d
Compare
|
I have reworked the names |
| @@ -13,6 +13,9 @@ | |||
| */ | |||
| class Erfurt_Exception extends Exception { | |||
|
|
|||
| const DEFAULT_ERROR = 0; | |||
There was a problem hiding this comment.
Add documentation for the usage of the error codes.
| @@ -13,4 +13,5 @@ | |||
| */ | |||
| class Erfurt_Store_Exception extends Erfurt_Exception | |||
| { | |||
| const NO_ERROR = 20; | |||
There was a problem hiding this comment.
Move all error code definitions to Erfurt_Exception
| @@ -896,7 +896,7 @@ public function getStore() | |||
| try { | |||
| $this->_store->checkSetup(); | |||
| } catch (Erfurt_Store_Exception $e) { | |||
| if ($e->getCode() != 20) { | |||
| if ($e->getCode() != Erfurt_Store_Exception::NO_ERROR) { | |||
There was a problem hiding this comment.
Rename this exception to actually reflect its role.
| @@ -327,10 +342,10 @@ protected function _getNormalizedErrorCode() | |||
| switch($this->_dbConn->getConnection()->errno) { | |||
| case 1062: | |||
| // duplicate entry | |||
| return 1000; | |||
| return Erfurt_Store_Exception::NO_ERROR; | |||
| @@ -327,10 +342,10 @@ protected function _getNormalizedErrorCode() | |||
| switch($this->_dbConn->getConnection()->errno) { | |||
| case 1062: | |||
| // duplicate entry | |||
| return 1000; | |||
There was a problem hiding this comment.
This method should be changed to boolean output.
| @@ -296,10 +308,10 @@ protected function _getNormalizedErrorCode() | |||
| switch($this->_dbConn->getConnection()->errno) { | |||
| case 1062: | |||
| // duplicate entry | |||
| return 1000; | |||
| return Erfurt_Store_Exception::NO_ERROR; | |||
Remove commented out code and apply coding standards of Erfurt_Store_Adapter_EfZendDb
shinobu
force-pushed
the
feature/Error_Codes
branch
from
5a6296d
to
2c0a4c1
Compare
…ltException and EarlyExitException as they are part of the Parser
shinobu
force-pushed
the
feature/Error_Codes
branch
from
2c0a4c1
to
72606a6
Compare
|
done and rebased onto your pull request @white-gecko |
This pull request adds error codes for all Exceptions except for
NoViableAltExceptionandEarlyExitExceptionas those both are used only in the Sparql Parser (and I doubt messing with it would be a good idea)I created the following Error Codes:
DEFAULT_ERRORfor 0 as 0 is the default output from thegetCode()function of Exceptionsand the places where it appears most likely only wanted to use the third optional parameter for creating an Exception.
DATABASE_ERRORandPARSER_ERRORfor -1 as all those files used that number as general exception for the class (most likely only to use the third optional parameter as well)FILTER_ERRORfor 1 for the same reason asDATABASE_ERRORandPARSER_ERRORNO_ERRORfor 20 as the Exceptions with this number are ignored