Here is simplified version of one method i have.
Still look very complex
How would one refactor this insanity?
protected function isTextValid()
{
if( $this->data['allow_num'] ){
if( $this->data['allow_space'] ){
if( preg_match( '#^[a-zA-Z0-9\s]$#', $this->val ) ){
return true;
}
else{
$this->messages = foo ? foo : bar;
return false;
}
}
else{
if( preg_match( '#^[a-zA-Z0-9]$#', $this->val ) ){
return true;
}
else{
$this->messages = foo? foor : bar;
return false;
}
}
}
else{
if( $this->data['allow_space'] ){
if( preg_match( '#^[a-zA-Z\s]$#', $this->val ) ){
return true;
}
else{
$this->messages = foo ? foor : bar;
return false;
}
}
else{
if( preg_match( '#^[a-zA-Z]$#', $this->val ) ){
return true;
}
else{
$this->messages = foo ? foo: bar;
return false;
}
}
}
}
I try refactor it using state pattern but up to no avail since im not quite familiar with the pattern.
This is what i done but quickyly abondon it.
interface ValidatorState{
public function isValid();
}
class AllowNumberAndSpace implement ValidatorState{
protected $context;
public function __construct(Context $context){$this->context = $context}
public function isValid(){
if( preg_match( .. ) ){
return true;
}
else{
$this->messages = foo ? foo : bar;
return false;
}
$this->context->setState(OtherState);
}
}
Class Context{
protected $state;
protected $allow_num_space_state;
public function __construct(){
$this->allow_num_space_state = new AllowNumberAndSpace($this);
$this->state = $this->allow_num_space_state;
}
public function isValid(){
return $this->state->isValid();
}
public function setState($state){$this->state = $state}
}
Obviously this only test first if
branch, how can i automatically check other branch too?
Im quite sure there is something wrong with my approach.
Is there a way to fix this state pattern to test for all if
branch?
Edited
What this method does is, it check whether $this->value
contain expected value based on configuration attribute stored in $this->data
Example $this->data = array('allow_num'=>true)
, if $this->value='a1'
it is consider valid
Example $this->data = array('allow_num'=>false)
, if $this->value='a1'
it is consider invalid
Is there a way to simplified this method?
preg_match
is different based on the if test – slier Nov 7 '13 at 18:54if/else
are faster than object instantiations and interfaces... – CodeAngry Nov 7 '13 at 18:55protected
orprivate
(as ininternal
). – CodeAngry Nov 7 '13 at 18:57