Transitioning to OOP in PHP

[jay.barnes]

Hi all,

I've been working with PHP, using procedural methods, for several years now, and have begun to explore the object-oriented side of it. While it didn't take me long to understand the potential advantages that come with working in this manner, I'm still having some trouble wrapping my head around doing the things I used to do procedurally in an object-oriented manner. A lot of the tutorials cover the basic applications of classes, properties, and methods, but I'm having trouble applying them to projects I'm working on.

For instance, I have a form, that I put a class together to validate it:

class formValidator {
    //set your properties for the class
    private var $user_input = array();
    protected var $response_code = 0;
    protected var $response_msg = "";

    public function set_validator($user_input){
        foreach ($user_input as $key => $value){
            //check to see if the field name contains "e-",
            //designating it as a required field
            if(strpos($key,"r-") !== FALSE) {
                //Check string length, if 0 then return empty field message
                if (strlen($value) === 0) {
                    $this->response_code = 1;
                    $this->response_msg = "Required field is empty!";
                }
            }
            //check to see if the field name contains "e-",
            //designating it as an e-mail field
            if(strpos($key,"e-") !== FALSE){
                //Check the datatype to confirm a string has been passed in
                if (is_string($value)&&($value != "")) {
                    //Regular expression pattern.
                    //Pattern breakdown:
                    //** [a-zA-Z0-9_] - any character between a-z, A-Z or 0-9
                    //** + - require one or more of the preceding item.
                    //** @{1} - Simply means 1 '@' symbol required.
                    //** [a-zA-Z]+ - any character between a-z, A-Z (1 or more required).
                    //** \.{1} - Single '.' required. Backslash escapes the '.'
                    //** [a-zA-Z]+ - One or more of the these characters require

                    //get rid of spaces in the field
                    $value = str_replace(' ','',$value);
                    $pattern = "/^[a-zA-Z0-9_]+@{1}[a-zA-Z]+\.{1}[a-zA-Z]+/";
                    //If the regex pattern does not match the value,
                    //set error code and error message
                    if (!(preg_match($pattern, $value))) {
                        $this->response_code = 1;
                        $this->response_msg .= "<BR>Entered e-mail address is not valid!";
                    }
                }
            }
        }
    }
    public function get_validator(){
            return $this->response_code;
            return $this->response_msg;
    }

Which I instantiate on the page, after a form is submitted:

if ($_POST["submit"]) {
    $validator = new formValidator();
    //$message = new message();  (gonna use this later...at least I plan to)

    // validate the form
    $validator->set_validator($_POST);
    $validator->get_validator();
}

Now, based on the validation status of the form, I want to create a class that echoes the message resulting from the validation, however, I'm not sure how to get the results from the validator class passed to a "message" class (I'm also trying to keep the 'message' class versatile enough to handle more than just form validation - I'd like to use it to display results of a successful form submission, user alerts, etc (I think that follows the concept of 'loose coupling', correct?).

I hope I've accurately described (as well as provided functional code to describe) my situation, here, and I welcome any feedback or suggestions you guys may have out there...

-Jay

[ryantroop]

for sure, this is broken:

public function get_validator(){
        return $this->response_code;
        return $this->response_msg;
}

once you return out of a method, you're done. It does not continue to process.

Other than that.. not sure what you are not understanding... If you get a response from your class, take the value and pass it to a method or instantiation of your message class... ??

[DaveAmour]

Hi

I'm not a PHP developer but do have good OOP skills.

Loose coupling is certainly key.

Have your validation class return an instance of ValidationResult (a class you will define unless there already is one in PHP?). This could be an instance of a class which has a boolean IsValid property and a colletion of string messages.

Don't have any html tags in your messages as you can reuse this class again and again - maybe in another app which doesn't use html.

SOLID is an acronym of good OO practice. The S stands for Single Responsibility principle which essentially just means that a class should only do one type of thing. In my programming language which is C# I have a ValidationResult class like this:

    public class ValidationResult
    {
        public bool IsValid
        {
            get
            {
                return !ValidationMessages.Any();   
            }
        }

        public List<string> ValidationMessages { get; set; }

        public ValidationResult()
        {
            ValidationMessages = new List<string>();
        }
    }

Hope that helps.

[DaveAmour]

PS - here is the ValidationResult being used:

        public ValidationResult ValidateAddRebateRate(Rebate rebate, DateTime fromDate, DateTime? toDate, decimal rate)
        {
            var validationResult = new ValidationResult();

            //1. The fromDate cannot be before the start date of the rebate
            if (fromDate.Date < rebate.StartDate.Date)
            {
                validationResult.ValidationMessages.Add("The from date cannot be before the start date of the rebate to which it belongs.");
            }

            //2. If we have a to date then it must be after the from date
            if (toDate.HasValue && toDate.Value.Date < fromDate)
            {
                validationResult.ValidationMessages.Add("The to date must be after the from date.");
            }

            //3. If this is the first rebate to be entered, then the start date must be on or before the rebate start date
            if (rebate.RebateRates.Count == 0 && fromDate > rebate.StartDate)
            {
                validationResult.ValidationMessages.Add("Since this is the first rate to be added to this rebate, the from date must be on or before the rebate start date. The actual rebate start date is ");
            }

            //4. If there are already existing rates, then the from date must be greater than the max existing from date of all rates apart from those with a status of rejected
            if (rebate.RebateRates.Any(r => r.Status != RebateRateStatus.Rejected) && fromDate <= rebate.RebateRates.Where(r => r.Status != RebateRateStatus.Rejected).Max(r => r.FromDate))
            {
                validationResult.ValidationMessages.Add("The from date must be after the from dates of all existing rates (not counting those which have been rejected).");
            }

            //5. If there are no existing rates and a toDate is supplied then this is not allowed - supress on the UI alsp
            if (toDate.HasValue && !rebate.RebateRates.Any())
            {
                validationResult.ValidationMessages.Add("You cannot supply a to date when entering the very first rate.");
            }

            //6. The rate must not be greater than the MaxRebateRate as configured in the Company Model
            if (Math.Abs(rate) > rebate.RebateTypeDefinition.Company.MaxRebateRate)
            {
                validationResult.ValidationMessages.Add("The rate can be no greater than {0}.  This is configured at Company level.");
            }

            //7. Make sure it is not the same as the previous rate otherwise it is pointless
            var mostRecentRate = rebate.RebateRates.OrderByDescending(r => r.FromDate).FirstOrDefault();

            if (mostRecentRate != null && mostRecentRate.Rate == rate)
            {
                validationResult.ValidationMessages.Add("The rate is the same as the previous rate so makes no sense.");
            }

            return validationResult;
        }

[broj1]

This code:

public function get_validator(){
    return $this->response_code;
    return $this->response_msg;
}

should be probably broken into two methods:

 public function get_response_code() {
    return $this->response_code;
}

public function get_response_msg() {
    return $this->response_msg;
}

I have been using OOP approach for several years now, and must say that it helps a lot. I can easily reuse classes I have written in past, it is easy to modify existing functionalities, it is also easy to organize team work and the project code is a piece of cake to maintain. I have learnt a lot by studying some open source examples and on my own mistakes (especially designing functionalities covered by each class). Some good code examples can be found on PHPclasses and I have also looked at some on the PEAR site.

[jkon]

Let's brake down what you are trying to do. You are trying to validate form inputs. There are many many ways with OOP to do this but let choose one that is near to your original approach.

Create an abstract class (lets call it FormValidatorAbstract) , having as constructor an array lets call it data that sets a protected property (lets call it data also) that in use will be the $_POST.

FormValidatorAbstract would then have two more protected properties responseMessage and responseCode (could be an object as well) (you don't have any reason to use the var clause) , with getters and setters (setters could be protected, but getters should be public) . Also FormValidatorAbstract would also have an abstract function lets call it validate().

FormValidatorAbstract also have validation functions like isEmail() , isInteger() , isUrl() , isDomain() , isEmpty() and so on that gets a piece of data make a validation and returns true or false.

It is your form , so you know the fields (there is no reason to use prefixes) , so the ContactFormValidator should extend FormValidatorAbstract and in its validate() function should do the logic to validate contact form fields using its parent validation functions setting the responseMessage and responseCode.

Then who ever instantiated the ContactFormValidator can getResponseCode() or getResponseMessage().

You could have also a Validate Utils class with static functions to perform common validations like isInteger() , this is a good approach for really simple and common things that don't change over the period of time , but if the validation has a business logic has meaning to be kept inside the FormValidatorAbstract.

[jay.barnes]

Thanks for the replies and suggestions, all - it gives me a lot of food for thought, but it doesn't quite answer my original question. I think I might be able to phrase it better:

Understanding using "extends" creates a parent-to-child "one to one" relationship, how would I go about creating a "many-to-one" relationship between classes or methods?

[pritaeas]

I want to create a class that echoes the message resulting from the validation, however, I'm not sure how to get the results from the validator class passed to a "message" class (I'm also trying to keep the 'message' class versatile enough to handle more than just form validation - I'd like to use it to display results of a successful form submission, user alerts, etc (I think that follows the concept of 'loose coupling', correct?)

You can use dependency injection to pass an instance of your message class to your form validator. If you want a more loosely coupled system, then you can use an interface in between, so the validator doesn't have to know everything about the message formatting class.

Understanding using "extends" creates a parent-to-child "one to one" relationship, how would I go about creating a "many-to-one" relationship between classes or methods?

Are you referring to a class maintaining a list/array of different objects?

[jay.barnes]

What was looking to do was essentially ""message" class extends classA, and classB, and classC, and classD,...."

However, the more I look at the proper implementaton of extending classes, I think I'm looking at it the wrong way (from what I see, extending a class is only used to extend and augment the fundamental purpose of the parent class, not to just call some other class and use its properties)...so, it looks like I'm wondering how to "call" properties from many other classes into one class.....does that make more sense?

I want this message class to be a generic, reusable class where I can get output from any other class to bring up a message that I can use anywhere on my site, for any reason...I want it to return error messages back to the user, bring up reminders, notify them of content updates, and almost any other single-instance communication you can think of bringing to a user's attention....

Thanks again for all your suggestions!

[pritaeas]

extending a class is only used to extend and augment the fundamental purpose of the parent class

Correct.

Sounds to me you're looking at it the wrong way. The message class should not be able to read from all classes (which would make it dependent on all classes), but it should be passed to those classes, to format the messages for them. That, or a communication between them, either routed through your code, or some kind of event. How I see it is something like this:

<?php
interface IMessageFormatter
{
    public function FormatMessage($message);
}

class MessageFormatter implements IMessageFormatter
{
     public function FormatMessage($message)
     {
         return "<div class='message'>$message</div>";
     }
}

class DoSomething
{
    private $messageFormatter;

    public function __construct($formatter)
    {
        $this->messageFormatter = $formatter;
    }

    public function Run()
    {
        $error = 'Something went wrong';

        if ($this->messageFormatter != null)
        {
            $error = $this->messageFormatter->FormatMessage($error);
        }

        throw new Exception($error);
    }
}

$msgFormatter = new MessageFormatter();
$doSomething = new DoSomething($msgFormatter);
$doSomething->Run();
?>

Another way would be to format the message in your script, instead of passing the interface to the class.

I want it to return error messages back to the user, bring up reminders, notify them of content updates, and almost any other single-instance communication you can think of bringing to a user's attention....

This class does GUI work and should not be dependent on classes that do only business logic. In this case I would use:

<?php
class MessageFormatter
{
     public function HandleMessage($message)
     {
         echo "<div class='error'>$message</div>";
     }
}

class DoSomething
{
    public function Run()
    {
        $error = 'Something went wrong';
        throw new Exception($error);
    }
}

$msgFormatter = new MessageFormatter();
$doSomething = new DoSomething();
try
{
    $doSomething->Run();
}
catch (Exception $e)
{
    $msgFormatter->HandleMessage($e->getMessage());
}
?>

Building one giant whale of a class that does a whole lot of things is generally not a good idea. The communication part is indeed one class, but what goes into those messages (IMO) should be generated by different classes built for different purposes. It also would make it easier to maintain/re-use one small class, instead of one very large one.

I wrote about OOP a while back, perhaps it might still be of some use.

[jay.barnes]

Heh...oddly enough, I've been reading and referring back to that article alongside this discussion thread! :-)

I was hoping that my message class could take two pieces of output from other classes: The error code (0, 1, or 2), and a short message string.

The error code would determine the formatting of the message back to the user (should the text be in red and bold-faced, as a critical error? Or black as simply an informational message?), and the message string, which would be just the content. Every class that would call that message would just need those two elements. I was looking to keep it simple, but versatile...

[pritaeas]

So basicly if you pass the code to the MessageFormatter in the example you kinda have what you are looking for?

[jay.barnes]

I think.....maybe? I'll have to play around with it a bit to figure it all out....I'm picking up on OO basics, but sometimes, I have to really turn some ideas over in my head for a while (sometimes a few days) just to wrap my brain around them...it all comes down to being too dumb to know when to quit! :-)

[jay.barnes]

Success!

I took some of your suggestions, pritaeas, and managed to cobble something together that worked (omitting comments for brevity's sake):

class formValidator {
    var $response_code = 0;
    var $response_msg = "";
    var $pwd = array();

    public function set_user_input($user_input){
        foreach ($user_input as $key => $value){
            if(strpos($key,"p-") !== FALSE) {
                //verify that password and confirm password fields match
                $pwd[] = $value;
            }
            if(strpos($key,"r-") !== FALSE) {
                //Check string length, if 0 then return empty field message
                if (strlen($value) === 0) {
                    $this->response_code = 2;
                    $this->response_msg .= "<LI>Required field, ".$key." is empty!";
                }
            }
            if(strpos($key,"e-") !== FALSE){
                if (is_string($value)&&($value != "")) {
                    $value = str_replace(' ','',$value);
                    $pattern = "/^[a-zA-Z0-9_]+@{1}[a-zA-Z]+\.{1}[a-zA-Z]+/";
                    if (!(preg_match($pattern, $value))) {
                        $this->response_code = 1;
                        $this->response_msg .= "<LI>Entered e-mail address is not valid!";
                    }
                }
            }
        }
        if ($pwd[0] != $pwd[1]) {
            $this->response_code = 1;
            $this->response_msg .= "<LI>Entered passwords do not match!";
        }
    }
    public function get_user_input(){
        $msg_params = array();
        $msg_params["code"] = $this->response_code;
        $msg_params["msg"] = $this->response_msg;
        return $msg_params;
    }
}
class MessageBox {
    var $color = "";
    var $msg = "";
    public function set_message($msg_params) {
        if ($msg_params["code"] == '0') {
            $this->color = "#00FF00";
        }
        elseif ($msg_params["code"] == '1') {
            $this->color = "#FF0000";
        }
        elseif ($msg_params["code"] == '2') {
            $this->color = "#FFFF00";
        }
        $this->msg = $msg_params["msg"];
    }
    public function get_message() {
        return "<div style=\"background-color:".$this->color."\">".$this->msg."</div>";
    }
}

And used as:

if ($_POST["submit"] == "Create Account") {
    $msg = new MessageBox();
    $validator = new formValidator();
    $validator -> set_user_input($_POST);
    $msg_params = $validator->get_user_input();
    $msg -> set_message($msg_params);

}

Thanks again for your assistance!

[diafol]

Having a lot of conditionals within a method is usually not that great as they're difficult to maintain, but it seems to work for you. Perhaps an easier method would be to use an array and check that.
Also scope your properties appropriately (private vs protected vs public). If they are not going to be accessible outside the class or to any other class, use private. If they are only accessed from another class, use protected.

class MessageBox {
    var $color = "";
    var $msg = "";
    public function set_message($msg_params) {
        if ($msg_params["code"] == '0') {
            $this->color = "#00FF00";
        }
        elseif ($msg_params["code"] == '1') {
            $this->color = "#FF0000";
        }
        elseif ($msg_params["code"] == '2') {
            $this->color = "#FFFF00";
        }
        $this->msg = $msg_params["msg"];
    }
}

Could be rewritten:

class MessageBox 
{
    private $color = "#00FF00"; //default
    private $msg = "";
    private $codes = array("#00FF00","#FF0000","#FFFF00");

    public function set_message($msg_params)
    {
        if(in_array($msg_params['code'],array_keys($this->codes)))
            $this->color = $this->codes[$msg_params['code']];
        $this->msg = $msg_params["msg"];
    }
}

Just a caveat though - array functions tend to be a bit slow, so it may be fractionally slower - but I haven't tested. However, you do get easier maintainability. All changes to be done in a property rather than have to mess with conditional branches.

You could even set up class constants to hold these values and pass a flag to the set_message, e.g. COOL_MSG, WARM_MSG, HOT_MSG (you get the idea)

[jay.barnes]

Interesting way to look at it- thanks diafol!

I'll run through your suggestions, if only just to at least get an understanding of what's going on in your code (I'm always looking for more efficient ways to accomplish what I'm trying to do).