Return to Revisions

Not yet implemented

It's unsafe to just have a comment like this:

// will be implemented later

At least add a TODO (so that an IDE can collect all of these):

// TODO will be implemented later

Or better yet, throw an exception:

throw new Exception('Not implemented');

If your code gets bigger and bigger, it's easy to overlook one or two of your original comments, and then your data will not be filtered correctly.

if-else return true-false

Again, whenever you have an if/else statement that only returns true/false, just return the statement inside the if. So for example

if( $pass != $cPass )
    return false;
else
    return true;

would become

return $pass === $cPass;

Notice also the === instead of ==.

The same goes for this:

if( $this->errorFlag == 0 ){
    return true;
}
else{
    return false;
}

write it like this instead:

return $this->errorFlag == 0;

End-of-Function comments

I think that these only clutter up the code, I would get rid of them.

Inside a function, // endif type comments are a sign of bad code. If you have so many if-statements that you cannot see where each ends, you probably have too many. Try to get rid of some or extract some functionality into a separate method.

Switch or if-else

Since you asked: I think that the switch looks better than an if-else.

Deeply nested checkRequiredData function

You have too many if-else in your checkRequiredData function which makes it really hard to read.

One way to solve this is to pull the checks to the top of the method like this:

function checkRequiredData($dataArray, $requiredFields = NULL) {
    if (count($dataArray) > 0) {
        $msg = "data array is empty";
        $this->setErrorMsg($msg);
        $this->errorFlag = 1;
        return false;
    }

    if ($requiredFields == NULL) {
        foreach ($dataArray as $key => $val) {
            if (empty($val)) {
                $msg = $key . REQUIRED_ERR;
                $this->setErrorMsg($msg);
                $this->errorFlag = 1;
            }
        }
        return false;
    }

    foreach ($dataArray as $key => $val) {
        if (array_key_exists($key, $requiredFields)) {
            $format = $requiredFields[$key];
            if (!empty($val) && $format != 'none') {
                if (!($this->dataFormatCheck($val, $format))) {
                    $msg = $key . VALIDATION_ERR;
                    $this->setErrorMsg($msg);
                    $this->errorFlag = 1;
                }
            } else {
                $msg = $key . REQUIRED_ERR;
                $this->setErrorMsg($msg);
                $this->errorFlag = 1;
            }
        }
    }
    return true;
}

Error Flag

You never set this to 0, is this intentional?

Also, you set this flag always directly after calling setErrorMsg. Just move $this->errorFlag = 1; inside setErrorMsg, so you don't have to manage it.

Unnecessary local variables

You do this very often:

$msg = $key . VALIDATION_ERR ;
$this->setErrorMsg($msg);

As you don't need msg afterwards, just call setErrorMsg directly:

$this->setErrorMsg($key . VALIDATION_ERR);