Validating booking information

Question

After recently reading up on and watching videos on OO PHP programming I have decided to convert my existing site into OO to gain some experience.

I have successfully (well, I think) converted a page into OOP and would like to know whether this is considered the proper way to implement OOP and whether this should be the way I complete the webpage or whether I need to rethink my design.

Here are the included files, a validate class file, and the "view" I guess, editedbooking.php. (In editedbooking.php a config file is included, which includes validate.php and database.php (my database connection file))

My class file - validatetest.php

<?php

class Validations {


public $date2;
public $typeofnumber;
public $phonenumber;


private function validatemobile($mobnumber) {
    $pattern =
            '/^[\(]?(\d{0,4})[\)]?[\s]?[\-]?(\d{3})[\s]?[\-]?(\d{3})$/';
    if (preg_match($pattern, $mobnumber, $matches)) {
        // we have a match, dump sub-patterns to $matches
        $number = $matches[0]; // original number
        $a_code = $matches[1];    // 4-digit area code
        $exchange = $matches[2];     // 3-digit exchange
        $ending = $matches[3];       // 3-digit number
        //    $extension = $matches[4];    // extension
        $this->phonenumber = "(".$a_code.") ".$exchange." ".$ending;
    } else {
        throw new Exception("Please Enter a valid Telephone Number");
    }
}

private function validatehome($number){
    $pattern =
        '/^[\(]?(\d{0,2})[\)]?[\s]?[\-]?(\d{4})[\s]?[\-]?(\d{4})$/';
    if (preg_match($pattern, $number, $matches)) {
        // we have a match, dump sub-patterns to $matches
        $number = $matches[0]; // original number
        $a_code = $matches[1];    // 2-digit area code
        $exchange = $matches[2];     // 4-digit exchange
        $ending = $matches[3];       // 4-digit number
 //         $extension = $matches[4];    // extension
        $this->phonenumber = "(".$a_code.") ".$exchange." ".$ending;
    } else {
        throw new Exception("Please Enter a valid Telephone Number");
    }
}

//method to check argument is set
public function is_set($arg1) {
    if (!isset($arg1) && ($arg1 == '')) {
        throw new Exception("Value {$arg1} is not set");
    }
}

//determine whether number is mobile or home phone
public function determine_number($number) {
    if ($number === "mobile") {
        $this->typeofnumber = "mobile";
    } elseif ($number === "home") {
        $this->typeofnumber = "home";
    }
}

//the method below validates a numbers length, length is set to 10 chars by default.
public function validatelength($arg1) {
    if ($this->typeofnumber == "mobile") {
        if (strlen($arg1) != 10) {
            throw new Exception("Your Mobile number is not the right length!");
        }
    } else if ($this->typeofnumber == "home"){
        if ((strlen($arg1) <= 7) || (strlen($arg1) >= 11)) {
            throw new Exception("The input \"$arg1\" is not the required length!");
        }
    } else {
        //error handling in here later
    }
}


//this method has to be passed after method "determinenumber"
public function validatenumber($phonenumber) {
    if ($this->typeofnumber == "mobile") {
        $this->validatemobile($phonenumber);
    } elseif ($this->typeofnumber == "home") {
        $this->validatehome($phonenumber);
    } else {
        throw new Exception("Please input a number type!");
    }
}

public function get_number() {
    return $this->phonenumber;
}



public function date_validate($date) {

    $fixingdate = $_REQUEST['date5'];
    $fixingdate = explode("-", $fixingdate);
    $year = $fixingdate[0];
    switch ($fixingdate[1]) {
        case 1:
            $month = "Janurary";
            break;
        case 2:
            $month = "Febuary";
            break;
        case 3:
            $month = "March";
            break;
        case 4:
            $month = "April";
            break;
        case 5:
            $month = "May";
            break;
        case 6:
            $month = "June";
            break;
        case 7:
            $month = "July";
            break;
        case 8:
            $month = "August";
            break;
        case 9:
            $month = "September";
            break;
        case 10:
            $month = "October";
            break;
        case 11:
            $month = "November";
            break;
        case 12:
            $month = "December";
            break;
    } // end of switch
    $month2 = $fixingdate[1];  //for different query (bloew)
    $day = $fixingdate[2];
    $subdate = "The ".$day."th of ".$month." ".$year;
    $this->date2 = "$year-$month2-$day";

    return $subdate;
}

public function servicemethod($service) {
    switch ($service) {
        case 1:
            $service = "Hair Extension - Consultation";
            break;
        case 2:
            $service = "Hair Extension - Application";
            break;

        case 3:
            $service = "Spray Tan";
            break;
        case 4:
            $service = "Eyelash Extensions";
            break;
    }
    return $service;
}

}

$validate = new Validations();

"view" page - editedbooking.php

<?php
require_once('calendar/classes/tc_calendar.php');

include ('includes/headerBK.html');
require_once('classes/config.php');


if ($session->is_logged_in() == TRUE) {    //main condition - if session isnt set then user isnt logged in.
$name = ucfirst($_SESSION['first_name']);
$userid = $_SESSION['user_id'];
$vars = $_SESSION;  //variables passed from login page to sesh var.
if(isset($_POST['submitted'])) {
    try {
        //validate the required fields are set
        $validate->is_set($_POST['phone']);
        $validate->is_set($_POST['numtype']);
        $validate->is_set($_POST['date5']);
        $validate->is_set($_POST['optone']);
        $validate->is_set($_POST['opttwo']);
        //determine the type of number being inputted i.e home or mobile
        $validate->determine_number($_POST['numtype']);
        //check the length of the entered phone number and date (just for an extra precaution)
        $validate->determine_number($_POST['numtype']); 
        $validate->validatelength($_POST['phone']);
        //validate number (has to run after "$validate->determinenumber")!
        $validate->validatenumber($_POST['phone']);
        //return the inputted items to a variables.
        $phone = $validate->get_number();
        $subdate = $validate->date_validate($_POST['date5']);
        $service = $validate->servicemethod($_POST['optone']);
        $location = ($_POST['opttwo']);
    } catch(Exception $e) {
        echo output_errormsg($e->getMessage() . "\n");  
    }

    if ($location && $phone && $subdate) {
        $database->autocommit(FALSE); //turn off mysqli auto commit
        $query1 = $database->use_db("INSERT INTO booking_info (location, ph_number) VALUES ('$location', '$phone')");
        if ($query1 == TRUE) {
            $query2 = $database->use_db("INSERT INTO bookings (user_id, date, pending) VALUES ($userid, '$validate->date2', 'yes')");
            if ($query2 == TRUE) {
                $finishpage = TRUE;
                $database->commit();
            } else {
                $database->rollback();
            }

        } else {
            //$database->rollback();
        }

        // first query - adding booking_info


    } else {
        echo output_errormsg("There was a problem with your request");
    }//END OF LOCATION PHONE AND DATE CONDITIONAL


}  //END OF MAIN SUBMITTED CONDITIONAL


if (isset($finishpage)) {  //if this is set then user has succesffully submitted the form already
    echo "Thank you $name, $subdate has been submitted for approval!";
    include('includes/footer.html');
    die();
}


} else {  //redirect as not logged in - MAIN SESSION CONDITIONAL
echo 'You are not logged in. Please click <a href="login.php">here</a>';
include('includes/footer.html');
die();


}  //end of session conditional*/


?>


<script language="javascript">
<!--
function setOptions(chosen)
{
var selbox = document.myform.opttwo;
selbox.options.length = 0;
if (chosen == " ") {
selbox.options[selbox.options.length] = new Option('Please select one of the options above first',' ');

}
if (chosen == "1") {
selbox.options[selbox.options.length] = new Option('On-Site','onsite');
selbox.options[selbox.options.length] = new Option('Mobile','mobile');
}
if (chosen == "2") {
selbox.options[selbox.options.length] = new Option('On-Site','onsite');
selbox.options[selbox.options.length] = new Option('Mobile','mobile');
}
if (chosen == "3") {
selbox.options[selbox.options.length] = new Option('On-Site','onsite');
selbox.options[selbox.options.length] = new Option('Mobile','mobile');
}
if (chosen == "4") {
selbox.options[selbox.options.length] = new Option('On-Site','onsite');
selbox.options[selbox.options.length] = new Option('Mobile','mobile');
}
}
-->
</script>


<img src="images/topwrapper.png" id="righttopimage"/>
<h1>Book Online</h1>



<div id="contactform">
<form name="myform" action="editedbooking.php" method="post">
    <fieldset><legend>Enter your information in the form below : </legend>
    <P><B>Contact Number:</b><input type="text" name="phone" size="40" maxlength="20" /></p>
    <p><b>Home Phone<input type="radio" name="numtype" value="home" /> Mobile<input type="radio" name="numtype" value="mobile" /></b></p>
    </p>


<table border="0"><tr><td><p><b>Preferred Time : </td><td><?php
$myCalendar = new tc_calendar("date5", true, false);
$myCalendar->setIcon("calendar/images/iconCalendar.gif");
//$myCalendar->setDate(date('d'), date('m'), date('Y'));
$myCalendar->setPath("calendar/");
$myCalendar->setYearInterval(2000, 2015);
$myCalendar->dateAllow('2008-05-13', '2015-03-01');
$myCalendar->setDateFormat('j F Y');
//$myCalendar->setHeight(350);
//$myCalendar->autoSubmit(true, "form1");
$myCalendar->setAlignment('left', 'bottom');
$myCalendar->setSpecificDate(array("2011-04-01", "2011-04-04", "2011-12-25"), 0, 'year');
$myCalendar->setSpecificDate(array("2011-04-10", "2011-04-14"), 0, 'month');
$myCalendar->setSpecificDate(array("2011-06-01"), 0, '');
$myCalendar->writeScript();
?>

</b></p></td></tr></table>


<p><b>Please select the service you require : </b></p>
<select name="optone" size="1" onchange="setOptions(document.myform.optone.options[document.myform.optone.selectedIndex].v    alue);">
<option value=" " selected="selected"></option>
<option value="1">Hair Extension - Consultation</option>
<option value="2">Hair Extensions - Application</option>
<option value="3">Spray Tan</option>
<option value="4">Eyelash Extensions</option>
</select><br /> <br />
<select name="opttwo" size="1">
<option value=" " selected="selected">Please select one of the options above first</option>
</select>

<input type="hidden" name="submitted" value="1">
</fieldset>

<div align="center"><input type="submit" name="submit" value="Submit My Information!" \></div>

</form>
</div>


<?php


include ('includes/footer.html');

?>

Answer 1

Firstly, let me say, Michael made very good comments which I agree with.

Minor Comments

1. The first thing I want to see in a class are the important parts. private methods do not help me interact with the class, so I always write them at the end of my class definitions.

2. Relying on user input can be dangerous:

   public function date_validate($date) {

   $fixingdate = $_REQUEST['date5'];
   $fixingdate = explode("-", $fixingdate);
   $year = $fixingdate[0];
   switch ($fixingdate[1]) {
   

   Someone could modify the request and you wouldn't get date5 as you expected.

3. Creating a global object after your class definition is a bad idea.

   $validate = new Validations();

   Create the object where you are going to use it (i.e editedbooking.php)

Major Comment - Is this OO?

Unfortunately this isn't really OO. It is a good first attempt.

Why do I say that?

Can you think of what Validations represents in the real world? The closest thing I can think of is that it is a procedure for validating data. Defining an object by its procedure is a way of still thinking in procedural terms.

You can see this in the servicemethod function. Your object is used as a procedure rather than a method. You pass in a value and return another one without referring to the object at all. The same happens with date_validate.

What real world object could have been used?

I understand you wanting to do validation, that is a good thing (though a little complicated). A booking or appointment type object might have been easier than a validation class.

How to make OO validations

I'm going to leave the implementation blank, but show you the kind of class structure I would use.

I would normally use an interface, but I'm going to skip that considering you are just starting OO. First the base class which is abstract:

abstract class Validator
{
    /** Basic validator.
     *  @param required \bool Whether the value must be set.
     */
    public function __construct($required = true) {};

    /** Return whether the data passed in is valid according to the validator.
     *  @param data \mixed The data to validate.
     */
    abstract public function isValid($data);
}

Notice how a common feature is recorded in the base class (whether the value is required). This feature is shared with all derived classes.

Now, a derived class which can do specific validation:

class PhoneValidator extends Validator
{
    /** Return whether the data passed in is valid according to the validator.
     *  @param data \mixed The data to validate.
     */
    public function isValid($data) {};   
}

By creating a number of different validation objects you could loop through all of your validations and ensure that they are all valid.

// Phone not required, Service implicity required, Date required.
$validations = array(new PhoneValidator(false),
                     new ServiceValidator(),
                     new DateValidator(true));

foreach ($validations as $validation)
{
   if (!$validation->isValid())
   {
       // Code for validation failure.
   }
}

The real difference is that these classes now represent real objects rather than straight method calls.

Answer 2

Design and style

As a general remark - choose your naming convention and use it consistently.

Validation

Public properties are always a bad thing

public $date2;
public $typeofnumber;
public $phonenumber;

I see just three public methods in validator (see below) All others are internal details of the validator implementation. These methods should return false at unsuccessfull validation.

public function getValidNumber($number, $typeOfNumber='mobile')
public function getValidDate($date)
public function getValidService($service)

View page

Defenetely, long way to "view" if you meant MVC. As for now, it has either too much PHP, or too much HTML. Depends on what it is: rather view, or may be template. It has "model"-related parts as well.

It is too big to review it as it is (as volonteer)....

General notes

Security

You trust too much to globals like $_SESSION and $_POST. I believe you should add more verifications of their content.

Programming

You use exceptions as message carriers. It is not common use. Exception is, well, exception from normal program flow, so it may take a lot of resources, for example, and you will have very expensive message transfer system.

Quite contrary, try catch is always a good thing around DB operations, but you don't do it.

swith in date_validate - don't you think it may be converted into array with boundary validation?

public function is_set($arg1) - seems to be too close to empty(). What does this statement mean:

if (!isset($arg1) && ($arg1 == ''))

I believe it is a bug.

validatehome and validatemobile differ in regexp only. Consider code reuse.