OOP in php5 Help

[Vbabiy]

<?php
class vlaidate
{
	private $_error = array();

function vlaidateEmail($email,$name)
{
	$trimed = trim($email);
	if (!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,8})$", $trimed))
	{
		$this->_error[$name] = $name.' Is not formated Correctly';
		return false;
	}
}
function vlaidateNumeric($num,$name)
{
	$trimed = trim($num);

	if (!is_numeric($trimed)) 
	{
		$this->_error[$name] = $name.' is not numeric';
	} 
	else 
	{
		return true;
	}		

}
function vlaidateTextOnly($text,$name)
{
	$trimed = trim($text);
	if (preg_match("/\d/", $trimed))
	{
   		$this->_error[$name] = $name.' is not only text it includes numeric charcters';
		return false;
    }

}		
function vlaidatePhonenumber($num,$name)
{
	$trimed = trim($num);
	if (!is_numeric($trimed)) {
		$this->_error[$name] = $name.' is not numeric';
	} else {		
		if ((strlen($trimed) < 10) || (strlen($trimed) > 10))
		{
			$this->_error[$name] = $name.'is not a vlaid Phone Number';
		}
	}
}
function strLenght($text,$name,$min = 0,$max)
{
	$newtext = trim($text);
	$strlength = strlen($newtext);
	if ($strlength > $max)
	{
		$this->_error[$name] = $name.' should only be '.$max.' characters long';
		return false;
	} elseif ($strlength <= $min) {
		$this->_error[$name] = $name.' is not long enough';
		return false;
	}

}
function removeHTML($text,$name)
{
	$trimed = trim($text);
	$newtext = strip_tags($trimed);
	if (empty($newtext)) 
	{
		$this->_error[$name] = $name.' is empty';
		return false;	
	} 
	else 
	{
		return $newtext;
	}


}
function getError()
{
	return $this->_error;
}
}




//Class AD
class ad extends vlaidate 
{
	private $_title;
	private $_summary;
	private $_description;
	private $_seller;


function __construct()
{

	//Defeine Constants for Title Lenghts
	define('TITLE_LENGTH_MIN',0);
	define('TITLE_LENGTH_MAX',10);

	//Define Constants for Summary Lenghts
	define('SUMMARY_LENGTH_MIN',0);
	define('SUMMARY_LENGTH_MAX',100);
}
function setTitle($title)
{
	$this->_title = $title;
	$this->strLenght($this->_title,'Title',TITLE_LENGTH_MIN,TITLE_LENGTH_MAX);		
}

function setSummary($summary)
{
	$this->_summary = $summary;
	$this->strLenght($summary,'Summary',SUMMARY_LENGTH_MIN,SUMMARY_LENGTH_MAX);
}

function setDescription($description)
{
	$this->_description = $description;
}

function setSeller($seller)
{
	$this->_seller = $seller;
}

function getTitle ()
{
	return $this->_title;
}

function getSummary()
{
	return $this->_summary;
}

function getDescription()
{
	return $this->_description;
}

function getSeller()
{
	return $this->_seller;
}

}

$ad = new ad();
$ad->setTitle('Title');
$ad->setSummary('Hello This is a great car i dont k');
$ad->vlaidateEmail('[e-mail]','Eamil');
$ad->vlaidatePhonenumber('5188783299','Home Phonenumber');
$ad->vlaidateTextOnly('asfasfsdfsd','TEST');
$ad->removeHTML('<h1>afslfjksla</h1>','HTML');
print_r($ad->getError());
?>

Learning OOP and hoping some on can give me some in site on how to even make these class better(more Secure, MORE OOP style... Any thing)

[Shrike]

You could split each validation method into it's own class. That way you can select only the validators necessary to perform the required validation. Also adding a new validator is a simple matter of writing a new class. No need to touch the existing one(s).

You could also benefit by using a common interface for validators, which would allow you to use features of polymorphism in your validation. By writing to an interface rather than an implementation you increase the reusability of the classes.

abstract class Validator
{
  private $error;
  abstract public function validate();
  abstract public function getError();
}
class EmailValidator extends Validator
{
  public function validate( $name, $string )
  {...}
  public function getError()
  {...}
}
$validators[] = new EmailValidator( 'email', '[e-mail]' );
$validators[] = new TextOnlyValidator( 'text', 'Some text' );
foreach( $validators as $v )
{
  if( !$v->validate() )
  {
    $errors[] = $v->getError();
  }
}

This type of abstraction is summarised by the Strategy design pattern.

Another pointer is to be wary of using inheritance. It should be used to specialise a class rather than simply provide it with functionality. Consider using composition or aggregation instead. Composition allows classes to be loosely coupled. Inheritance is by definition tight coupling. In this case it may not be appropriate at all. Your ad class probably doesn't need to know how it's validated. Validator certainly has nothing to do with ad.

$ad = new ad();
$ad->setEmail( '[e-mail]' );
$validators[] = new EmailValidator( $ad->getEmail() );
foreach( $validators as $v )
{
  if( !$v->validate() )
  {
    $errors[] = $v->getError();
  }
}

Hope this helps a bit, there is alot to learn though 🙂

[Vbabiy]

Thanks for all the help that reallly cleared up alot of things 🙂

[Vbabiy]

Is this better?
Am i get the point?
If not please help me 🙂

<?php
//Interface for the Vlaidator Classes
interface vlaidator
{    

    public function vlaidate($string);
    public function getErrors();

}

//emailVlaidator Implents interface Vlaidator
//Vlaiding Emails
class emailVlaidator implements vlaidator 
{
    private $_error;

public function vlaidate($email)
{
    $trimed_email = trim($email);
    if (!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,8})$", $trimed_email))
    {
        $this->_error = 'Email Is not formated Correctly';
        return false;
    }
}

public function getErrors()
{
    return $this->_error;
}
}

//Vlading for string to make sure they only include Text charchters
class textOnlyVlaidator implements vlaidator 
{
    private $_errors;
    private $_name;

public function setName($name)
{
    $this->_name = $name;
}

public function vlaidate($text)
{
    $trimed = trim($text);
    if (preg_match("/\d/", $trimed))
    {
           $this->_errors = $name.' is not only text it includes numeric charcters';
        return false;
    }
}

public function getErrors()
{
    return $this->_errors;
}
}

//Vlading for numeric
class numericVlaidator implements vlaidator 
{
    private $_errors;
    private $_name;

public function setName($name)
{
    $this->_name = $name;
}

public function vlaidate($num)
{
    $trimed = trim($num);

    if (!is_numeric($trimed)) 
    {
        $this->_error[$name] = $this->_name.' is not numeric';
    } 
    else 
    {
        return true;
    }    
}

public function getErrors()
{
    return $this->_errors;
}

}

class textLengthVlaidator implements vlaidator
{
    private $_errors;
    private $_name;

private $_min = 0;
private $_max;

public function setName($name)
{
    $this->_name = $name;
}

public function setMax($max)
{
    $this->_max = $max;
}

public function setMin($min)
{
    $this->_min = $min;
}
public function vlaidate($text)
{
    $newtext = trim($text);
    $strlength = strlen($newtext);
    if ($strlength > $this->_max)
    {
        $this->_errors= $this->_name.' should only be '.$this->_max.' characters long';
        return false;
    } elseif ($strlength <= $this->_min) {
        $this->_errors = $name.' is not long enough';
        return false;
    }
}

public function getErrors()
{
    return $this->_errors;
}
}

class phoneNumberVlaidator implements vlaidator 
{
    private $_errors;

public function vlaidate($number)
{
    $trimed = trim($number);

    if (!(strlen($trimed) == 10) || !(is_numeric($trimed))) 
    {
        $this->_errors = 'The Phone number is incorrect';
    }
}

public function getErrors()
{
    return $this->_errors;
}
}
?> 

[thorpe]

Yeah... more to the point. However, if you made vlaidator an abstract class instead of an interface, you could put the getErrors() implimentation within it. This way you dont need to redefine it in each individual class.

abstract class vlaidator
{    

    private $_error;
    public function vlaidate($string);
    public function getErrors()
    {
        return $this->_error;
    }       

}

[Vbabiy]

Not sure how that would work here is what i have so far:

<?php
//Interface for the Vlaidator Classes
abstract class vlaidator
{	
	private $_error;

abstract  public function vlaidate($string);
public function getErrors()
{
	return $this->_error;
}

}

//emailVlaidator Implents interface Vlaidator
//Vlaiding Emails
class emailVlaidator extends vlaidator 
{
	private $_error;

public function vlaidate($email)
{
	$trimed_email = trim($email);
	if (!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,8})$", $trimed_email))
	{
		$this->_error = 'Email Is not formated Correctly';
		return false;
	}
}

}

$new = new emailVlaidator();
$new->vlaidate('my@sadfcom');
echo $new->getErrors();
?>

When i run this it creates to $_error var both private and when the $this->getErrors() is run from the abstract class it try to pull from that abstract class private var to echo it out which is empty the error is really stored in the emailVlaidator class...

[Shrike]

You might want to use protected rather than private properties.

[Vbabiy]

OK here are my end results after fixing up my class:

Let me know what you think..

<?php

abstract class validator
{	
	protected $_error;

abstract  public function validate();
public function getErrors()
{
	return $this->_error;
}

}

//Vlaiding Emails
class emailvalidator extends validator 
{	
	private $_name;
	private $_email;

public function __construct($name,$email)
{
	$this->_name = $name;
	$this->_email = $email;

}

public function validate()
{
	$trimed_email = trim($this->_email);
	if (!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,8})$", $trimed_email))
	{
		$this->_error = $this->_name.' is not formated Correctly';
		return false;
	} else {
		return true;
	}
}

}

//Vlading for string to make sure they only include Text charchters
class textOnlyvalidator extends validator 
{
	private $_name;
	private $_text;

public function __construct($name,$text)
{
	$this->setName($name);
	$this->setText($text);
}
private function setText($text)
{
	$this->_text = $text;
}
private function setName($name)
{
	$this->_name = $name;
}

public function validate()
{
	$trimed = trim($this->_text);
	if (preg_match("/\d/", $trimed))
	{
   		$this->_error = $this->_name.' is not only text it includes numeric charcters';
		return false;
    } else {
    	return true;
    }
}

}

//Vlading for numeric
class numericvalidator extends validator 
{
	private $_name;
	private $_num;

public function __construct($name,$num)
{
	$this->setName($name);
	$this->setNum($num);

}
private function setName($name)
{
	$this->_name = $name;
}

private function setNum($num)
{
	$this->_num = $num;
}

public function validate()
{
	$trimed = trim($this->_num);

	if (!is_numeric($trimed)) 
	{
		$this->_error = $this->_name.' is not numeric';
		return false;
	} 
	else 
	{
		return true;
	}	
}

}

class textLengthvalidator extends validator
{
	private $_text;
	private $_name;

private $_min = 0;
private $_max;

public function __construct($name,$text,$max,$min = 0)
{
	$this->setName($name);
	$this->setText($text);
	$this->setMax($max);
	$this->setMin($min);
}
private function setText($text)
{
	$this->_text = $text;
}
private function setName($name)
{
	$this->_name = $name;
}

private function setMax($max)
{
	$this->_max = $max;
}

private function setMin($min)
{
	$this->_min = $min;
}
public function validate()
{
	$newtext = trim($this->_text);
	$strlength = strlen($newtext);
	if ($strlength > $this->_max)
	{
		$this->_error= $this->_name.' should only be '.$this->_max.' characters long';
		return false;
	} elseif ($strlength <= $this->_min) {
		$this->_error = $this->_name.' should be atleast '.$this->_min.' characters long';
		return false;
	} else {
		return true;
	}
}
}

class phoneNumbervalidator extends validator 
{
	private $_number;
	private $_name;

public function __construct($name,$num)
{
	$this->setName($name);
	$this->setNumber($num);
}

private function setName($name)
{
	$this->_name = $name;
}

private function setNumber($num)
{
	$this->_number = $num;
}
public function validate()
{
	$trimed = trim($this->_number);

	if (!(strlen($trimed) == 10) || !(is_numeric($trimed))) 
	{
		$this->_error = $this->_name.' is incorrect';
	} else {
		return true;
	}
}
}


$class[] = $new = new emailvalidator('User Email','[e-mail]');
$class[] = $text = new textOnlyvalidator('User name','Vitaly');
$class[] = $num = new phoneNumbervalidator('Home Phone Number',5127852155);
$class[] = $len = new textLengthvalidator('Summary','adflaskjflasjflskajflsadjflsdkjklsjflasjkdflasdjflsk aklsjfls adkjflsdjk f',100);
$class[] = $num2 = new numericvalidator('Numbers','123');

foreach ($class	as $v)
{
	if (!$v->validate()) {
		echo $v->getErrors().'<br />';		
	}



}
?>