Jump to content

Recommended Posts

Keep playing if you have, if you haven't just play with it, it's really simple if you put the register & login on two pages. Once you see whats happening you can combine them.

 

I removed the register form but still with the same problem, I am misunderstanding what removing this web form is achieving?

What do you have now?

 

PS. You haven't played with it much, I made my last post 55+- minutes ago.

 

I tried a few ideas which also didnt work by moving things about. I could call the function before any HTML output but how would I get the form to display where I want it rather than the top of the page before the header etc.

 

 

At the moment I have

 

signin.php

 

<?php
require_once('_coreFunctions.php'); // this is where DB functions are e.g. connection etc
require_once('_signInFunctions.php');
require_once('header.php'); // this has HTML inside of it e.g. head tag etc

?>
<div id="contentBoxWrapper">
<div id="contentBoxTop">
    	<div id="contentBoxTopInner"><a href="/" class="firstLink">Home</a> <span class="breadcrumbDivider">></span> Sign In<hr /></div>
    </div>
    <div id="contentBoxContent">
                <div id="signInFormWrapper">
                <h1>Sign In</h1>
                <?php sif_loginForm(); ?> // this is where I call the login form function......
                <br />
                <a href="resetpassword">Forgotten password?</a>
                </div>
            </div>
    <div id="contentBoxBottom"></div>
</div>
<?php require_once('footer.php'); ?>

 

 

_signInFunctions.php

 

<?php
require_once('_coreFunctions.php');

function sif_loginForm()
{
$displayForm = true;
$error = "";
$username = "";

if(isset($_POST['signInSubmit']))
{
	$username = mysqli_real_escape_string(cf_dbConnect(), $_POST['username']);
	$password = mysqli_real_escape_string(cf_dbConnect(), $_POST['password']);

	$query = mysqli_query(cf_dbConnect(), "call sp_checkLoginDetails('{$username}', '{$password}')");
	if(mysqli_num_rows($query) == 1)
	{
		$_SESSION['isLoggedIn'] = true;
		while($row = mysqli_fetch_object($query))
		{
			$_SESSION['firstName'] = $row->firstName;
		}
		mysqli_query(cf_dbConnect(), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')");
		header("Location: ."); // this is my redirect part
	}
	else
	{
		$error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>";

		if($username == "username...")
		{
			$username = "";
		}
	}

}

if($displayForm)
{
	echo "
		<form name=\"signInForm\" id=\"signInForm\" method=\"post\" action=\"\">
				<label for=\"username\">Username:</label><br />
				<input type=\"text\" name=\"username\" id=\"signInFormUsername\" class=\"formField\" value=\"{$username}\" /><br /><br />
				<label for=\"password\">Password:</label><br />
				<input type=\"password\" name=\"password\" id=\"signInFormPassword\" class=\"formField\" /><br /><br />
				<input type=\"submit\" id=\"signInSubmit\" name=\"signInSubmit\" value=\"Login\" />
		</form>
		<br />
		{$error}
	";
}
}


?>

Ok, I have came up with an idea, using the principle of seperating out the business logic from the layout.

 

Rather than echoing out the form code from within the function, I can call the function at the top of the page before any HTML e.g. before the header.php include. The fuction will still have the if - post so will still work.

 

But.... Using this principle if I use the same idea for a contact form as an example and I then want to hide the form once submitted and show a response message of 'thank you for contacting us', how would I achieve this using this new method as the form code would always be hard coded into the html page?

php processes from top to bottom.

 

I'd do something like this but it probably has some errors

 

<?php
require_once('_coreFunctions.php'); // this is where DB functions are e.g. connection etc
require_once('_signInFunctions.php');

$displayForm = true;
$error = "";
$username = "";

if(isset($_POST['signInSubmit']))
{
	$username = mysqli_real_escape_string(cf_dbConnect(), $_POST['username']);
	$password = mysqli_real_escape_string(cf_dbConnect(), $_POST['password']);

	$query = mysqli_query(cf_dbConnect(), "call sp_checkLoginDetails('{$username}', '{$password}')");
	if(mysqli_num_rows($query) == 1)
	{
		$_SESSION['isLoggedIn'] = true;
		while($row = mysqli_fetch_object($query))
		{
			$_SESSION['firstName'] = $row->firstName;
		}
		mysqli_query(cf_dbConnect(), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')");
		header("Location: ."); // this is my redirect part
	}
	else
	{
		$error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>";

		if($username == "username...")
		{
			$username = "";
		}
	}

}

require_once('header.php'); // this has HTML inside of it e.g. head tag etc
?>
<div id="contentBoxWrapper">
<div id="contentBoxTop">
<div id="contentBoxTopInner"><a href="/" class="firstLink">Home</a> <span class="breadcrumbDivider">></span> Sign In<hr /></div>
</div>
<div id="contentBoxContent">
<div id="signInFormWrapper">
<h1>Sign In</h1>
<?php if (isset($error)) {echo $error;} ?>
<form name=\"signInForm\" id=\"signInForm\" method=\"post\" action=\"\">
<label for=\"username\">Username:</label><br />
<input type=\"text\" name=\"username\" id=\"signInFormUsername\" class=\"formField\" value=\"{$username}\" /><br /><br />
<label for=\"password\">Password:</label><br />
<input type=\"password\" name=\"password\" id=\"signInFormPassword\" class=\"formField\" /><br /><br />
<input type=\"submit\" id=\"signInSubmit\" name=\"signInSubmit\" value=\"Login\" />
</form>
  <br />
<a href="resetpassword">Forgotten password?</a>
</div>
</div>
<div id="contentBoxBottom"></div>
</div>
<?php require_once('footer.php'); ?>

I then want to hide the form once submitted and show a response message of 'thank you for contacting us', how would I achieve this using this new method as the form code would always be hard coded into the html page?

 

use an if() condition

I then want to hide the form once submitted and show a response message of 'thank you for contacting us', how would I achieve this using this new method as the form code would always be hard coded into the html page?

 

use an if() condition

 

Thanks for the above code, that is what I was thinking towards, when you say use an IF(), i am guessing I would need to put the if condition in the signin.php around the form which is back in normal HTML, so wouldnt that be mixing layout with logic again

 

e.g.

 

<?php
require_once('_coreFunctions.php'); // this is where DB functions are e.g. connection etc
require_once('_signInFunctions.php');

$displayForm = true;
$error = "";
$username = "";

if(isset($_POST['signInSubmit']))
{
	$username = mysqli_real_escape_string(cf_dbConnect(), $_POST['username']);
	$password = mysqli_real_escape_string(cf_dbConnect(), $_POST['password']);

	$query = mysqli_query(cf_dbConnect(), "call sp_checkLoginDetails('{$username}', '{$password}')");
	if(mysqli_num_rows($query) == 1)
	{
		$_SESSION['isLoggedIn'] = true;
		while($row = mysqli_fetch_object($query))
		{
			$_SESSION['firstName'] = $row->firstName;
		}
		mysqli_query(cf_dbConnect(), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')");
		header("Location: ."); // this is my redirect part
	}
	else
	{
		$error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>";

		if($username == "username...")
		{
			$username = "";
		}
	}

}

require_once('header.php'); // this has HTML inside of it e.g. head tag etc
?>
<div id="contentBoxWrapper">
<div id="contentBoxTop">
<div id="contentBoxTopInner"><a href="/" class="firstLink">Home</a> <span class="breadcrumbDivider">></span> Sign In<hr /></div>
</div>
<div id="contentBoxContent">
<div id="signInFormWrapper">
<h1>Sign In</h1>
<?php if (isset($error)) {echo $error;} ?>

<?php
if(!isset($_POST['submit']))
{
echo "
<form name=\"signInForm\" id=\"signInForm\" method=\"post\" action=\"\">
<label for=\"username\">Username:</label><br />
<input type=\"text\" name=\"username\" id=\"signInFormUsername\" class=\"formField\" value=\"{$username}\" /><br /><br />
<label for=\"password\">Password:</label><br />
<input type=\"password\" name=\"password\" id=\"signInFormPassword\" class=\"formField\" /><br /><br />
<input type=\"submit\" id=\"signInSubmit\" name=\"signInSubmit\" value=\"Login\" />
</form>";
}
?>
  <br />
<a href="resetpassword">Forgotten password?</a>
</div>
</div>
<div id="contentBoxBottom"></div>
</div>
<?php require_once('footer.php'); ?>

Now we're talking. :)

 

What you can do, and what I actually recommend to do, is to still use the function to validate input and generate the form. That way you can leverage the return construct to exit the function early, and as such cut down a whole lot on the nesting. Something which will make your code a lot easier to read, and thus more maintainable.

Now we're talking. :)

 

What you can do, and what I actually recommend to do, is to still use the function to validate input and generate the form. That way you can leverage the return construct to exit the function early, and as such cut down a whole lot on the nesting. Something which will make your code a lot easier to read, and thus more maintainable.

 

So have one function that will check if form is posted (like the one I already have) and then also have another function to echo the form out?

 

Do you have a very basic example on what you mean by the return construct to exit?

I can give you an basic example of the flow that I'm using, in pseudo-code, perhaps that will give you some pointers. Using a contact form as the base, as it's a very simple and common issue.

Do note that I'm using a template engine though, to select and show the proper HTML code. In addition to that I'm running the following code inside a function, so this is just the specific code for the contact form itself.

 

 // First off we check if something has been submitted.
if (submit) {
    // If it has, retrieve and validate all user input immediately.
    validate (username);
    validate (email);
    validate (message);

    // Make sure all fields validated.
    if (!validated) {
        // If something failed validate, make sure user knows what and doesn't have to type in any of the details again.
        template->add_warning (Failed fields);
        template->repopulate (form);
        template->add_form ();

        // Return to the calling function, so that the template data can be parsed and printed.
        return;
    }

     // Make sure input it safe for use in e-mails.
     escape_output ('mail');

    // Try to send the email, and redirect the user to the correct page depending upon the success of the email.
     if (send_mail ()) {
         header ('location: success');
         die ();
    }

    template->add_error ('Email failed');
    template->repopulate (form);
    template->add_form ();
}

// Nothing has been posted.
template->add_form ();

Ok, I have refactored my code just to try and get this working then I will start sorting the functions out to display the form.

 

The problem is now that I am losing variables due to scope, I DONT want to create globals as I hear they are not good.

 

Basically I need to use the $displayForm variable that is defined in the function to be accessible outside of the function and on the signin.php page, the same would go for the $error variable too?

 

any ideas on the best way to do this?

 

Thanks

 

signin.php

 

<?php
require_once('_coreFunctions.php');
require_once('_signInFunctions.php');
sif_checkLoginForm();  // calling the check form function where $displayForm is defined
require_once('header.php');

?>
<div id="contentBoxWrapper">
<div id="contentBoxTop">
    	<div id="contentBoxTopInner"><a href="/" class="firstLink">Home</a> <span class="breadcrumbDivider">></span> Sign In<hr /></div>
    </div>
    <div id="contentBoxContent">
                <div id="signInFormWrapper">
                <h1>Sign In</h1>
                <?php
				if($displayForm)  // this variable isnt available due to scoping problems
				{
					echo "
							<form name=\"signInForm\" id=\"signInForm\" method=\"post\" action=\"\">
									<label for=\"username\">Username:</label><br />
									<input type=\"text\" name=\"username\" id=\"signInFormUsername\" class=\"formField\" value=\"{$username}\" />
									<br /><br />
									<label for=\"password\">Password:</label><br />
									<input type=\"password\" name=\"password\" id=\"signInFormPassword\" class=\"formField\" /><br /><br />
									<input type=\"submit\" id=\"signInSubmit\" name=\"signInSubmit\" value=\"Login\" />
							</form>
							<br />
							{$error}
						";
				}
			?>
                <br />
                <a href="resetpassword">Forgotten password?</a>
                </div>
            </div>
    <div id="contentBoxBottom"></div>
</div>
<?php
require_once('footer.php');
?>

 

 

_signInFunctions.php

 

<?php

require_once('_coreFunctions.php');

function sif_checkLoginForm()
{
$username = "";
$displayForm = true;    // defined here, dont want to use GLOBALS

if(isset($_POST['signInSubmit']))
{
	$username = mysqli_real_escape_string(cf_dbConnect(), $_POST['username']);
	$password = mysqli_real_escape_string(cf_dbConnect(), $_POST['password']);

	$query = mysqli_query(cf_dbConnect(), "call sp_checkLoginDetails('{$username}', '{$password}')");
	if(mysqli_num_rows($query) == 1)
	{
		$_SESSION['isLoggedIn'] = true;
		while($row = mysqli_fetch_object($query))
		{
			$_SESSION['firstName'] = $row->firstName;
		}
		mysqli_query(cf_dbConnect(), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')");
		header("Location: .");
		die();
	}
	else
	{
		$error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>";

		if($username == "username...")
		{
			$username = "";
		}
	}

}
}


?>

Use return, or if you need multiple variables from the function, passing by reference.

 

The passing by reference worked a treat!

 

my amended code

 

signin.php

 

<?php
require_once('_coreFunctions.php');
require_once('_signInFunctions.php');
sif_checkLoginForm($displayForm, $username, $error);   // I am passing in the variables defined in _signInFuntions.php
require_once('header.php');

?>
<div id="contentBoxWrapper">
<div id="contentBoxTop">
    	<div id="contentBoxTopInner"><a href="/" class="firstLink">Home</a> <span class="breadcrumbDivider">></span> Sign In<hr /></div>
    </div>
    <div id="contentBoxContent">
                <div id="signInFormWrapper">
                <h1>Sign In</h1>
                <?php
				if($displayForm)
				{
					echo "
							<form name=\"signInForm\" id=\"signInForm\" method=\"post\" action=\"\">
									<label for=\"username\">Username:</label><br />
									<input type=\"text\" name=\"username\" id=\"signInFormUsername\" class=\"formField\" value=\"{$username}\" />
									<br /><br />
									<label for=\"password\">Password:</label><br />
									<input type=\"password\" name=\"password\" id=\"signInFormPassword\" class=\"formField\" /><br /><br />
									<input type=\"submit\" id=\"signInSubmit\" name=\"signInSubmit\" value=\"Login\" />
							</form>
							<br />
							{$error}
						";
				}
			?>
                <br />
                <a href="resetpassword">Forgotten password?</a>
                </div>
            </div>
    <div id="contentBoxBottom"></div>
</div>
<?php
require_once('footer.php');
?>

 

 

_signInFunctions.php

 

<?php
require_once('_coreFunctions.php');

$displayForm = true;   // define the variables that are needed on the signin.php page
$username = "";
$error = "";

function sif_checkLoginForm(&$displayForm, &$username, &$error)      // pass in references to these variables so they can be accessed
{

if(isset($_POST['signInSubmit']))
{
	$username = mysqli_real_escape_string(cf_dbConnect(), $_POST['username']);
	$password = mysqli_real_escape_string(cf_dbConnect(), $_POST['password']);

	$query = mysqli_query(cf_dbConnect(), "call sp_checkLoginDetails('{$username}', '{$password}')");
	if(mysqli_num_rows($query) == 1)
	{
		$_SESSION['isLoggedIn'] = true;
		while($row = mysqli_fetch_object($query))
		{
			$_SESSION['firstName'] = $row->firstName;
		}
		mysqli_query(cf_dbConnect(), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')");
		header("Location: .");
		die();
	}
	else
	{
		$error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>";

		if($username == "username...")
		{
			$username = "";
		}
	}

}

}


?>

Note that passing by reference is something you should use sparingly, as it make the code harder to read. Most of the time it's better to return an array, and use list () to separate it into individual variables if you need that.

I've also taken the liberty to clean up your function a bit, to further showcase what I meant by my previous posts:

// Pass in references to these variables so they can be accessed 
function sif_checkLoginForm (&$displayForm, &$username, &$error) {
// Make sure the form has been submitted.
if (!isset ($_POST['signInSubmit'])) {
	// If not, exit early.
	return false;
}

// I suspect you want to use the database object here, and not (what looks like)
// starting a new connection for each call to mysqli_real_escape_string ().
$username = mysqli_real_escape_string (cf_dbConnect (), $_POST['username']);
$password = mysqli_real_escape_string (cf_dbConnect (), $_POST['password']);

// Check for error status first.
$query = mysqli_query (cf_dbConnect (), "call sp_checkLoginDetails('{$username}', '{$password}')");
if (mysqli_num_rows ($query) != 1) {
	$error = "<div id=\"formMessages\">Username or Password is incorrect. Please try again.</div>";

	if ($username == "username...") {
		$username = "";
	}

	// Exit early.
	return false;
}

$_SESSION['isLoggedIn'] = true;
while ($row = mysqli_fetch_object ($query)) {
	$_SESSION['firstName'] = $row->firstName;
}

mysqli_query (cf_dbConnect (), "call sp_updateLoginDateTime('{$_SESSION['firstName']}')");
header ("Location: .");
die ();
}

 

Anyway, glad we could help.

Note that passing by reference is something you should use sparingly, as it make the code harder to read. Most of the time it's better to return an array, and use list () to separate it into individual variables if you need that.

 

Anyway, glad we could help.

 

Ok, I have switched to returning an array and using list as suggested and it works too :) it also like you suggests keeps my code readable.

 

Thank you all who contributed to this, it is now working perfect :)

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.