[Mail] Avis sur ma première classe en PHP5

Version imprimable

En guise d'exercice je me suis amusé à coder une classe extremement simple
et je vous l'accorde sans doute assez inutile.Toutefois j'aurai souhaité savoir
ce que vous en pensiez d'un point de vue strictement formel, est-ce correctement programmé, quels sont les défauts de cette classe ? (hormis le fait qu'elle ne soit pas d'une grande utilitée).

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
class MyMail{
    private $exp;
    private $dest;
    private $cont;
    private $subject;
 
    public function setExp($exp){
        if($this->verifAdMail($exp)){
            $this->exp=$exp;
        }
    }
    public function setDest($dest){
        if($this->verifAdMail($dest)){
            $this->dest=$dest;
        }
    }
    public function setCont($cont){
        if($this->verifCont($cont)){
            $this->cont=$cont;
        }
    }
    public function setSubject($subject){
        if($this->verifSubject($subject)){
            $this->subject=$subject;
        }
    }
    public function SendMyMail(){
        if(!empty($this->exp)
        &&!empty($this->dest)
        &&!empty($this->cont)
        &&!empty($this->subject)){
 
               $this->cont .= '\n Envoyé par : '. $this->exp;
 
               if(mail($dest,$subject,$cont)){
                      $this->message = "Votre mail a bien été envoyé.";
               }else{
                      throw new Exception("Une erreur est survenue durant l'envoi du mail");
               }
 
        }else{
            if(empty($this->exp)){
                throw new Exception("Le champ de l'expediteur est vide");
            }
            if(empty($this->dest)){
                throw new Exception("Le champ du destinataire est vide");
            }
            if(empty($this->cont)){
                throw new Exception("Votre message est vide");
            }
            if(empty($this->subject)){
                throw new Exception("Votre sujet est vide");
            }
        }
    }
 
//---------------------------------------------------------------------------------------
// Les fonctions private maintenant : 
//---------------------------------------------------------------------------------------
 
    private function verifAdMail($exp){
         if($this->is_email($exp)&&$this->mail_exist($exp)){
             return TRUE;
         }else{
            throw new Exception('Mail invalide, veuillez corriger votre adresse e-mail');
        }
    }
    private function is_email($exp){
        return preg_match("/^(\w|-|\.)+@((\w|-)+\.)+[a-z]{2,6}$/i",$exp);
    }
    private function mail_exist($exp){
        list($compte,$domaine)=split("@",$exp,2);
        if(!checkdnsrr($domaine,"MX")&&!checkdnsrr($domaine,"A")){
            return FALSE;
        }
        return TRUE;
    }
//-----------------------------------------------------------------------------------------
    private function verifSubject($subject){
        //On pourra plus tard créer un parametre tableau 
        //listant les sujets "interdits" etc....
        $subject = htmlspecialchars($subject);
        return $subject;
    }
 
//------------------------------------------------------------------------------------------    
    private function verifCont($cont){
        $cont = htmlspecialchars($cont);
        return $cont;
    }
}

Ah j'oubliais, il s'agit d'envoyer un email en procedant auparavant à une verification de chaque paramètres.

24/05/2008, 11h51
mic79

Salut,

Je la trouve pas mal la classe ;-)

Attention, il n'y a pas de commentaire dans le code.

@+

Mic
24/05/2008, 15h59
Tsilefy

Bonjour,

Juste histoire de pinailler :D
- Chacun de tes setters sont des fonctions void, et ne retournent rien en cas d'échec. Je vois que plus tard, tu effectues une vérification sur les propriétés pour contrôler s'ils ne sont pas vides, avec exceptions à la clef. Ça répond sans problème à tes soucis actuels, je pense. Mais quid si plus tard, tu utilises la classe dans un autre contexte, où l'utilisateur aura la possibilité de modifier / corriger les données ? Dans ce cas, tu ne sauras pas si la seconde opération (la modification) aura été effectuée sans problèmes, et aucune exception ne sera lancée puisque la propriété n'est pas vide. un simple if / else return true /false résoudrait ce souci.
- Il peut être préférable de mettre les propriétés en protected plutôt que private. Comme ça, si d'autres classes héritent de cette classe, ils auront accès aux propriétés.

Version un peu corrigée :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
<?php
 
 class MyMail{
    private $exp;
    private $dest;
    private $cont;
    private $subject;
    private $confirmation ="Nothing sent yet";
 
 
    public function __construct(){}
 
 
    public function setExp($exp){
        if($this->verifAdMail($exp)){
            $this->exp=$exp;
        }
    }
    public function setDest($dest){
        if($this->verifAdMail($dest)){
            $this->dest=$dest;
        }
    }
    public function setCont($cont){
        if($this->verifCont($cont)){
            $this->cont=$cont;
        }
    }
    public function setSubject($subject){
        if($this->verifSubject($subject)){
            $this->subject=$subject;
        }
    }
    public function sendMyMail(){
 
        if(($this->verifAdMail($this->exp)&&$this->verifAdMail($this->dest)
            &&$this->verifCont($this->cont)&&$this->verifSubject($this->subject))){
 
               $this->cont .= '\n Send by : '. $this->exp;
 
               if(mail($this->dest,$this->subject,$this->cont)){
                      $this->confirmation=  "Your email have been sent";
               }else{
                      throw new Exception("An error occured during sending process ");
               }     
        } //Pas de else les messages d'erreurs concernés sont déjà géré par les 
           //exceptions des méthodes appellées dans le if.
    }
    public function getConfirmation(){
        return $this->confirmation;
    }
 
//---------------------------------------------------------------------------------------
// Les fonctions private maintenant : 
//---------------------------------------------------------------------------------------
 
    private function verifAdMail($exp){
         if($this->is_email($exp)&&$this->mail_exist($exp)){
             return TRUE;
         }
    }
    private function is_email($exp){
        if(!empty($exp)){
            if(filter_var($exp,FILTER_VALIDATE_EMAIL)==FALSE){
 
                throw new Exception('Invalid mail address');
            }else{
                return TRUE;
            }
 
        }else{
            throw new Exception('Expeditor is empty');
        }
    }
    private function mail_exist($exp){
        list($compte,$domaine)=split("@",$exp,2);
        if(!checkdnsrr($domaine,"MX")&&!checkdnsrr($domaine,"A")){
           throw new Exception('Invalid email'); //Ne fonctionne pas sous windows.
        }
        return TRUE;
    }
//-----------------------------------------------------------------------------------------
    private function verifSubject($subject){
        if(!empty($subject)){
        //On pourra plus tard créer un parametre tableau 
        //listant les sujets "interdits" etc....
        $subject = htmlspecialchars($subject);
        return TRUE;
        }else{
            throw new Exception('subject is empty');
        }
    }
 
//------------------------------------------------------------------------------------------    
    private function verifCont($cont){
        if(!empty($cont)){
        $cont = htmlspecialchars($cont);
        return TRUE;
        }else{
            throw new Exception('text area is empty');
        }
 
    }
}
 
 
?>