Is this try catch a good practice? Will I be encountering problems should I implement this? Considering also unit test.
public function index($user)
{
try {
$message = $this->_validate($user);
} catch (MyCustomException $e) {
$message = $e->getMessage();
}
return $message;
}
private function _validate($user)
{
if ($user != 'test') throw new MyCustomException('Invalid User');
return 'Valid User';
}
UPDATED (Added another example) How about if I retrieve data from SQL then when no records were retrieve, throw an exception.
public function retrieveData($args)
{
$codeTable = $model->getCodeTables('code_tables', $args);
if (empty($codeTable)) throw new MyCustomException($this->getCustomMessage()['sysMsgDataDoesNotExists']);
// more codes here
return $codeTable;
}
public function index($args)
{
try {
$this->retrieveData($args);
// if data retrieve is not empty, execute codes here like insert, delete, update other records or other fancy logic
$message = 'success';
} catch (MyCustomException $e) {
$message = $e->getMessage();
}
return $message;
}
On the 2nd example, my goal here is to immediately go to catch when no data were retrieved. Instead of me doing something like this.
public function retrieveData($args)
{
$codeTable = $model->getCodeTables('code_tables', $args);
if (empty($codeTable)) return $this->getCustomMessage()['sysMsgDataDoesNotExists'];
// more codes here
return $codeTable;
}
public function index($args)
{
$data = $this->retrieveData($args);
if (is_array($data)) {
// if data retrieve is not empty, execute codes here like insert, delete, update other records or other fancy logic
$message = 'success';
} else {
$message = $data;
}
return $message;
}
You should only throw when you encounter a circumstance that can't be handled, rather than using throw in order to handle something.
In this case, a flag for true or false is appropriate.
public function index($user)
{
return isValid($user) ? 'Valid user' : 'Invalid user';
}
private function isValid($user)
{
return $user === 'test';
}
An example where throw makes sense if you're writing a function that requires a parameter to be passed, and that parameter isn't passed. That means the developer forgot to pass it, and the best way to let him know is to throw, so that everything stops.
function foo($a, $b) {
if (!$a || !$b) {
throw new Exception('Gotta have parameters, dude!');
}
// etc
}
That is not the intended use of exceptions and is bad practice. Exceptions are intended for those conditions that are not forseable and/or outside of the control of the current developer.
In your case you can predict that some users will not be 'test' users, otherwise why have the test. What you are doing here is to use an exception just to return an alternative message that you then echo. So you do not need to throw an exception, simply return an alternative message that indicates this.
Some would argue when using the database and having potential sql errors one should catch those and log them/bubble them up to the user.
Others would go for wrapping any external libraries within a try catch.
Personally I want to wrap all of my code in try catch and bubble up the exception ( throw them all the way to the controller ) where I would log them and if they're found handle output in a user friendly format.
The takeaway - they're simply a way to route out of your logic in a graceful manner if anything unpredictable occurs.
Here's an example of how I would wrap index to be consistent at all times:
public function index( $args )
{
// Init return.
$aReturn = array(); // Or object.
try
{
$aFetch = $this->retrieveData( $args );
if (!empty( $aFetch) )
{
$aReturn = $aFetch;
}
}
catch( Exception $oException )
{
// Optionally log exception or do nothing.
// log( $oException );
}
// Return will always be an array either with data or empty.
return $aReturn;
}
For the additional examples, I guess it depends on the context of what you're really querying.
For example:
If the query is some search then it's possible you don't get any results. Your wrapper/orm should just return an empty array and treat it as a normal situation. You can check the number of rows and decide what to do.
If the query is something like "what's the tax rate I should use on this date" and the application was supposed to have the information preloaded before it was used, then this is possibly a reason for an exception. But only from the get_tax_rate()-like function, not from the generic database querying code. (even then it may not need to be an exception if there's some user-exposed page for defining tax rates and "I don't have a tax rate, please define one" is a valid error message)
Any situation you cannot really control, like broken connections, timeouts, getting data that doesn't conform to your model, etc. are definitely reasons to throw exceptions.