im doing a a basic form where the user enters data, it has three php pages, one with the html stuff, second one with the a class called registration, the third one is called validation.php it has the error handlers.
if the user typed something wrong, i want to display an error on the same page, that’s why im using sessions, see in the html page if the session is set it should echo it, but it’s echoing it anyways, i don’t know why.
the html page:
<form action="registration.php" method="post">
<input type="text" placeholder="Email" name="email" required><br>
<input type="password" placeholder="password" name="password" required><br>
<input type="password" placeholder="Repeat password" name="password2" required><br>
<?php if(isset($_SESSION["noMatch"])); echo $_SESSION["noMatch"]; ?>
the validation.php
class Validation
{
function samePasswordCheck($password, $password2)
{
if ($password != $password2)
{
$_SESSION["noMatch"]="passwords don't match";
return false;
}
else return true;
the registration page
require 'validation.php';
if(isset($_POST['finishreg'])){
if (!empty('email') && !empty('password') && !empty('password2'))
{
$email = $_POST['email'];
$password = $_POST['password'];
$password2 = $_POST['password2'];
$obj = new Validation();
$first=$obj->emailFormatCheck($email);
$second=$obj->securePasswordCheck($password);
$third=$obj->samePasswordCheck($password, $password2);
if($first==true && $second==true && $third==true )
{
//send to database blah blah
}
i tried to cut the code so it only shows what matters, as you can see the “noMatch” is set if the passwords aren’t the same, so why is it getting echoed anyway? and is this a good practice? am i doing all of this the right way?, im trying to make this OOP, thank you.
Advertisement
Answer
Now you’re keeping the state of the errors, which is a step forward to your previous question.
As you’re now keeping the state even in the session, you need to manage it in there.
That is if you flag an input with an error and the next time the validation runs, it needs to unflag it if it was valid. Otherwise the error flag stays in the session despite it is now valid.
This normally is easier if you put these things apart. The one part is the validation and the other part is the session handling.
However for the simplicity of the example, this should show where the validation is missing:
function samePasswordCheck($password, $password2)
{
if ($password != $password2)
{
$_SESSION["noMatch"]="passwords don't match";
return false;
}
else return true;
The $_SESSION["noMatch"] is invalidated by setting it. But the else block is never validating it (e.g. by unsetting it).
This could be as simple as:
if ($password != $password2)
{
$_SESSION["noMatch"]="passwords don't match";
return false;
}
else
{
unset($_SESSION["noMatch"]);
return true;
}
But as written you don’t want to have session handling within the validation normally. It is baked too much together and already now creating you headaches (with doing too many things at once, it’s easy to loose control).
Instead you can make the validation routines return a boolean. That would also make them look nicer:
function samePasswordCheck(string $password, string $password2): bool
{
return $password !== $password2
}
You can then have a routine that binds the form-data and the session data so you have separated the validation logic from the state handling.
In the end you have more code, but you should have less places you need to visit to add new changes.
E.g. if there is an error comparing the passwords (like using weak comparison instead of strict comparison), only the validation function regarding the passwords is affected. Maybe that error has been made in all validation? Well easy to check as you have all validation functions next to each other.
As they all return bool, the rest of the code is not affected.