1

I want to create an email class with many functions and use this methods
I want some advice how to make this flexible and reliable as much as I can but another important feature is security to verify all the external input
I am going to past all my code that I made so far, this is my own work not copy paste or anything else.

This what I made so far

<?php

/*Email class 
 * 
 * @var $name;
    var $email;
    var $address;
    var $phone;
    var $city;
    var $message;

 */

class email{

    private $name;
    private $email;
    private $address;
    private $phone;
    private $city;
    private $message;


    function content(){
       if( iseet($_POST['submit'])){
        $this->name=  strip_tags($_POST['name']);    
        $this->email=  strip_tags($_POST['email']);
        $this->address=  strip_tags($_POST['address']);
        $this->phone=  strip_tags($_POST['phone']);
        $this->city =  strip_tags($_POST['city']);
        $this->message =  strip_tags($_POST['message']);
        }


    }

    function verify(){


     if(!ereg ("([a-zA-z]", $this->name)){

         $ename='Error the name contains  numbers or not permitted symbols';

     }



       } 





    function error(){



   }



   function send(){


   }    


}



?>

Any suggestion or expanation is welcomed Thank you.

6
Contributors
14
Replies
15
Views
5 Years
Discussion Span
Last Post by dany12
Featured Replies
  • From a high level, your class is trying to accomplish to much in a single place. Check out the concept of SOLID (http://www.freeklijten.nl/home/2012/03/23/SOLID-The-S-is-for-Single-responsibility) What you should ultimately end up with is a set of classes that allow you to work with emails in the most generic of ways. An Email … Read More

0
  1. Use preg_match instead of the deprecated ereg function.
  2. In function content there is a typo, it should be isset
  3. If you pass an array as parameter to content, then you are able to use other arrays than just $_POST
  4. In verify you set a variable. Either make this a class variable, or return it from the function. That way you can access the result to see what's wrong.

Edited by pritaeas

0

sorry didn't see that isset
I don't understand that part with the post are you suggesting to make something like this

$name=striptags($_POST['name']);

not to use a private variable for name?

I want to verify all the external input and then create an array with all the errors and read them from function error
Thank you for the good feedback

Edited by dany12

0

I have reviewed my code

<?php
/*Email class 
 * 
 * @var $name;
    var $email;
    var $address;
    var $phone;
    var $city;
    var $message;
 */
class email{

    function content(){
       if( isset($_POST['submit'])){
        $name=  strip_tags($_POST['name']);    
        $email=  strip_tags($_POST['email']);
        $address=  strip_tags($_POST['address']);
        $phone=  strip_tags($_POST['phone']);
        $city =  strip_tags($_POST['city']);
        $message =  strip_tags($_POST['message']);
        }
    }
    function verify(){
         if(!preg_match ("([a-zA-z]", $name)){
             $ename='Error the name contains  numbers or not permitted symbols';
             }

         if(!FILTER_VALIDATE_EMAIL,$email){
         $eemail = 'email is not valid';
         }


       } 
    function error(){
   }
   function send(){
   }    
}
?>
0

The private variables are good, you should use them for the error messages too in my opinion. The way you had it was fine.

0

Hi,

You can also use php filter_var function. I keep on forgetting about this function. You can use this function in your helper method.

Something like this..

private function verifyEmail($email) {

 ## returns true or false
return filter_var($email, FILTER_VALIDATE_EMAIL) && preg_match('/@.+\./', $email);
}

private function sanitizeString($tstring){
 ## this method will be use by others within the class

return trim(filter_var($tstring, FILTER_SANITIZE_STRING));  

} 
private function sanitizeUrl($url){
return filter_var($this->url, FILTER_SANITIZE_URL);
}

Edited by veedeoo: more info added

0

To use it within your class, you can pretty much do it like this.

 $email=  strip_tags($_POST['email']);
 if(!self::verifyEmail($email)){
 error = "email is not a valid email address";
 }
 ## we can also clean up the message using the helper function like this

  $message =  strip_tags($_POST['message']);

  return trim(self::sanitizeString($message));

There are many more ways I could show, but my keyboard is acting up again.

Edited by veedeoo: more info added

0

Thank you from what I seen you are suggesting to create a verification method for each external data but I was thinking to create a single class verify and get all errors in an array and display only the apropriate ones to the email form.
I used this in a login logout script and wondering If I can implemented here?

Thanks in adavanced.

0

Hi,

Not exactly though, you can insert them inside your class and call them within the class. This way you will only have to call one or two methods outside your class. That's the reason why I set its visibility to private on my exmaple.

You can also retrieve the methods returned data as an array if you wish to do so.

for example,

    public function doSomething(){
    $this->error = "";
    if(SomeCondition){
    $this->error ="condition A error";

    }

    else{
    $this->error .= "condition B error";
    ## otherwise error is set to default as empty

    }
    ## say, we want to call helper methods here to validate something, then we can do so by just using the self::doSomethingElse().
    ## example..
     if(!self::verifyEmail($email)){
$this->error .= "email is not a valid email address";
}
else{
$this->email = true;
## you can either insert to database or just return the boolean true above.
$this->error .= "email has been added";
}

## return the array from this method, for other methods to utilize

return array($this->error, $this->email);

}

and then, within the method utilizing the method above can be coded like this. This will and can be use outsde this class, whenever the class is instantiated anywhere in your site.

 public function getAllValidation(){
 $this->emailHelper = self::doSomething();
 $this->otherThings = self::functionOfOtherThings();
 $this->someOtherValidations = self::functionToValidateOtherThings();
 ## extract the data returned by doSomething.
 $this->error = $this->emailHelper[0];
 $this->validation = $this->emailHelper[1];

 ## extract the rest of the helpers 

 ## do some other things here..

 ## return all validations as an array.. it can be an error or an approval or boolean.

 return array($this->error,$this->validation, $this->otherThings, $this->moreValidationsHere);
 }

Again, if you want the function doSomething to be reusable within the class, then just set the visibility to private. However, there is NO iron clad here that we cannot set it to public and use the function doSomething outside the class. Of course we can call it somewhere else if we desire.. just set it to public.

There are many things you can add in your class, like checking if the user or email exist in the database, methods chaining is also a common practice in OOP..

Edited by veedeoo: added something

0

you lost me at the end I was just trying to make this simple use a error class to display errors in a div in the email form like in jquery

0

Check this, I tried to make it kinda object oriented.
I tried to do something else rather than the usual way I did this, not sure if it's good or bad.

I kinda didn't respect the Keep it Simple Stupid Logic :)

<?php 

class ContactForm{

    /**
      * @var array ('name', 'email_address', 'subject', 'message')
      */
    public $information = array();

    private $_is_valid_information = false;

    public $error_messages = array(
        'name'          =>  array(
                                'required'  =>  'This field is required'
                                ),
        'email_address' =>  array(
                                'required'  =>  'This field is required',
                                'invalid'   =>  'This is not a valid email address'
                                ),
        'subject'       =>  array(
                                'required'  =>  'This field is required'
                                ),
        'message'       =>  array(
                                'required'  =>  'This field is required'
                                )
    );

    public function __construct(){

    }

    /**
      * @param array $information
      * @return array $errors
      */
    public function setInformation($information){
        $this->information = $information;

        $this->_trimInformation();

        return $this->_validateInformation();
    }

    /**
      * @throws Exception
      * @return void
      */
    public function sendEmail(){
        if(empty($this->information))
            throw new Exception("Information not set");

        if(!$this->_is_valid_information)
            throw new Exception("Information is not valid");

        // Send an email.
    }

    /**
      * @return array - an array of errors
      */
    private function _validateInformation(){
        $errors = array();

        if(isset($information['name']) 
            && $information['name'] == ''){
            $errors['name'] = $this->error_messages['name']['required'];
            $this->_is_valid_information = false;
        }else{
            $this->_is_valid_information = true;
        }

        if(isset($information['email_address']) 
            && $information['email_address'] == ''){
            $errors['email_address'] = $this->error_messages['email_address']['required'];
            $this->_is_valid_information = false;
        }elseif(isset($information['email_address'])
            && !$this->_isEmailAddress){
            $errors['email_address'] = $this->error_messages['email_address']['invalid'];
            $this->_is_valid_information = false;
        }else{
            $this->_is_valid_information = ($this->_is_valid_information && true);
        }

        if(isset($information['subject']) 
            && $information['subject'] == ''){
            $errors['subject'] = $this->error_messages['subject']['required'];
        }else{
            $this->_is_valid_information = ($this->_is_valid_information && true);
        }

        if(isset($information['message']) 
            && $information['message'] == ''){
            $errors['message'] = $this->error_messages['message']['required'];
        }else{
            $this->_is_valid_information = ($this->_is_valid_information && true);
        }

        return $errors;
    }

    /**
      * @return void
      */ 
    private function _trimInformation(){
        foreach($this->information as $key=>$field){
            $this->information[$key] = $field;
        }
    }

    /**
      * @todo Make some real email address verify regex
      * @param string $email
      * @return bool
      */
    private function _isEmailAddress($email){
        return true;
    }

}

?>
0

Validating the text content looking for certain keywords (poker, insurance, pharma etc) or script tags and rejecting the message if they are found will reduce the amout of junk messages you get. Spammers are creatures of habit and nearly always try to write links as full code in the hope you will not notice where the link goes. In reality nobody codes links in contact forms, not even geeks and technophobes definitely do not write this sort of thing.

<a href="some.spam.pharmacy.site.com"> check this out </a>   

Any good web hosting provider will also have a security limit on the server mail() function, attempt to send more than say 100 messages in under ten minutes and the web host will flag the site as a source of suspected spam and disable it (ask your webhost for specific details). So if somebody keeps pushing the send button on your contact form they could easily crash your website without them needing any 'hacker' skills unless you do something to stop them.

Other usefull things to experiment with..
Automatically append the users message with information about their PC such as Operating system, browser type (IE,firefox,safari etc) and IP address. This can help troubleshoot users problems or be used to filter out spammers.

1

From a high level, your class is trying to accomplish to much in a single place. Check out the concept of SOLID (http://www.freeklijten.nl/home/2012/03/23/SOLID-The-S-is-for-Single-responsibility)

What you should ultimately end up with is a set of classes that allow you to work with emails in the most generic of ways. An Email object would represent an email and it's fields only. This should probably implement a standard interface so you can develop multiple email objects, e.g. PlainText, Html, Multi-part etc. These would extend a base class or implement some kind of inteface to ensure they have certain basic methods.

From there you would have an object that represents a mail server and is responsible for sending an Email Object. For example EmailTransport. Email transport might be setup to use different adapters that allow it to interact with multiple server types, sendmail, smtp, mock(for saving an email object to a file for testing), and mail(). This can be designed using the Adapter pattern.

For validation and filtering you would want two finds of objects, One that is able to deal with specific pieces of validation. Again you'd have multiple validators each with a single functionality. Filters would be setup the same way.

This is really high level and a lot to digest but these are some of the common things I see wrong with OOP code in the PHP world and I hope this helps to set you on the right path.

<?php

    namespace Daniweb\Email;

    interface EmailInterface
    {
        protected $to;
        protected $from;
        protected $subject;
        protected $body;

        /**
         *  Defines required methods
         *  @see http://php.net/manual/en/language.oop5.interfaces.php
         **/
    }
    ?>

    <?php

    namespace Daniweb\Email;

    abstract class EmailGeneric implements EmailInterface
    {
        /**
           ... implements required methods from inteface

         **/
    }
    ?>

    <?php
    namespace Daniweb\Email\Type;

    use Daniweb\Email\EmailGeneric;

    class PlainText extends EmailGeneric
    {
        /**
            ... adds plaintext functionality
        **/
    }
    ?>

    <?php
    namespace Daniweb\Email\Type;

    use Daniweb\Email\EmailGeneric;
    class Html extends EmailGeneric
    {
        /**
            ... adds html specific functionality
        **/
    }
    ?>

    <?php
    namespace Daniweb\Email;
    use Daniweb\Email\TransportInterface;
    use Daniweb\Email\EmailInterface;

    class Transport
    {
        public function __construct( TransportInterface $adapter )
        {
            /**
                stores the transport adapter
                **.
        }

        public function send( EmailInterface $email  )
        {
            /**
                uses the transport to send the email object, returns some kind of status
              **/
    }
    ?>


    <?php
    namespace Daniweb\Email\Transport;

    interface TransportInterface
    {
        /**
           ...defines transport methods that should be implemented
           **/
    }
    ?>

    <?php
    namespace Daniweb\Email\Transport\Adapter;

    use Daniweb\Email\Transport\TransportInterface;

    class Smtp implements TransportInterface
    {
        /**
           ... SMTP functionality

           **/
    }
    ?>

Usage:

<?php

    use Daniweb\Email\Type\Html;
    use Daniweb\Email\EmailTransport;
    use Daniweb\Email\Transport\Adapter\Smtp;
    use Daniweb\Email\Transport;

    $adapter = new Smtp( array() ); //Pass in smtp setting or use setters etc.
    $email = new Html( array() ); //Pass in html email values.

    $transport = new EmailTransport( $adapter );
    $result = $transport->send( $email );

    // $result stores some kind of result code etc.

    ?>

I'm sorry this turned out to be such a long post but I hope it helps you understand better ways to use OOP to separate responsibilities and functionality into much more manageable pieces. I didnt cover validation or filtering as these would be their own components that would be used on the values that are passed into an Email object.
I'm certain this is a bit overwhelming but I hope you, or sometone else, find some value in it.

*Notes:
This requires PHP 5.3
This does not provide any working code
This assumes you'd be using a PSR-0 compatabile autoloader etc.
Ending php tags should NOT be included in actual class files. (Only used to show where one file starts and ends)

0

mschroeder you know he doesn't actually know much about object oriented programming,
He has using php 4 sintax, and you came with.. hmm namespacing, interfaces .
And as Sogo7 said, this is a really "weak" part to build a class for it.

This question has already been answered. Start a new discussion instead.
Have something to contribute to this discussion? Please be thoughtful, detailed and courteous, and be sure to adhere to our posting rules.