Skip to content

Commit

Permalink
Correct replace_outside_quotes error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
treffynnon committed Nov 14, 2012
1 parent 318d7cd commit 47f6dea
Showing 1 changed file with 9 additions and 3 deletions.
12 changes: 9 additions & 3 deletions idiorm.php
Original file line number Diff line number Diff line change
Expand Up @@ -1459,8 +1459,9 @@ protected function _str_replace_outside_quotes(){
)* # Zero or more string chunks.
\z # Anchor to end of string.
/sx';
if (!preg_match($re_valid, $this->subject)) // Exit if string is invalid.
exit("Error! String not valid.");
if (!preg_match($re_valid, $this->subject)) {
throw new IdiormStringException("Subject string is not valid in the replace_outside_quotes context.");
}
$re_parse = '/
# Match one chunk of a valid string having embedded quoted substrings.
( # Either $1: Quoted chunk.
Expand All @@ -1487,4 +1488,9 @@ protected function _str_replace_outside_quotes_cb($matches) {
return preg_replace('/'. preg_quote($this->search, '/') .'/',
$this->replace, $matches[2]);
}
}
}

/**
* A placeholder for exceptions eminating from the IdiormString class
*/
class IdiormStringException {}

2 comments on commit 47f6dea

@tag
Copy link
Contributor

@tag tag commented on 47f6dea Nov 20, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is adding the new class IdiormStringException necessary? Perhaps use a one of PHP's built-in Exceptions instead, such as UnexpectedValueException or LogicException?

I mention this as one alternative, as the new exception class has implications for correcting autoloading (re j4mie/paris#32).

@treffynnon
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version 1.* of both Paris and Idiorm are intended as simple, easy to drop in ORM and active record classes. So your suggestion in j4mie/paris#32 will not be implemented until at best a version 2 release as it is a BC break to separate classes out for the sake of easier composer autoloading. Not everyone is using composer and certainly not for Idiorm and Paris as it has only just been released.

Whilst it is not strictly necessary to include a separate exception I like to be able to easily catch exceptions so this makes it much more explicit.

Please # to comment.