OOP simple contact form

Here is my Code. But I feel that my code isn't really that much object oriented.

There is a small piece of "styling" in it as well, the "message error" thing at the end.

Should that be in my class or should it go somewhere else?

senderName   = $_POST['senderName'];
$sendMail->senderEmail  = $_POST['senderEmail'];
$sendMail->recipient    = $_POST['recipient'];
$sendMail->copy     = $_POST['copy'];
$sendMail->subject  = $_POST['subject'];
$sendMail->message  = $_POST['message'];

$sendMail->sendMail();

}
?>

    
        Send E-mail
    

    
        From:Enter the sender's name in this field.
        " size="50" maxlength="125" />
    

    
        From E-mail:Enter the sender's e-mail address in this field.
        " size="50" maxlength="125" />
    

    
        Recipient:Enter the recipient's e-mail address in this field.
        " size="50" maxlength="125" />
    

    
        Carbon Copy:Send a copy to someone else? Enter another e-mail address here. Leave blank for no copy.
        " size="50" maxlength="125" />
    

    
        Subject:Enter a subject in this field.
        " size="50" maxlength="50" />
    

    
        
    

And this is the contact.class.php:

```
senderName != "") {
$this->senderName = filter_var($this->senderName, FILTER_SANITIZE_STRING);
if ($this->senderName == "") {
$errors .= '- Please enter a valid name!';
}
} else {
$errors .= '- You forgot to enter a name!';
}

if ($this->senderEmail != "") {
$this->senderEmail = filter_var($this->senderEmail, FILTER_SANITIZE_STRING);
if ($this->senderEmail == "") {
$errors .= '- Please enter a valid Email!';
}
} else {
$errors .= '- You forgot to enter an email!';
}

if ($this->recipient != "") {
$this->recipient = filter_

Here are some observations noticed:

For something to be OOP it needs to be somewhat reusable. As always use SOLID principles to achieve this.

Because you are using styling it broke some of the rules. Consider returning outputs such as errors and such to be pure text without styling and let the return handle it. Reason: what happens if you want to log the fail message and shoot it out internally or to a log file.

Your sendmail is a jack of all trade: it does the header, store the emails, AND checks for errors - this is procedural (start, middle end) - consider separating it into different functions of class (your original one and then have a separate class to do validations which you send as arguments).

Consider using a constructor. You initialize the object with settings and then reuse the function.

Another point to add: attributes (or variables from a class that pertains to the class) should never be never accessible to the "main" or elsewhere but to the class itself. Use accessor functions like get/set or magic functions _get _set instead. Classes are suppose to be encapsulated so that outside code cannot effected without going through a checker. I'm aware that its easier to just access it directly but you defeat the purpose of OOP in that sense.

Lastly, too many if/else renders the code too tight which is why I suggested the validation class - let that be the class to check on the arguments rather than the mailer itself.

StackExchange Code Review Q#44551, answer score: 6