71367d98df2bf14cbf704e7741eb402d

be easier?

(login and Registration in ajax)

<?php
ob_start();
include_once "include/config.php";

$login = safe_chars($login);
$pass = safe_chars($pass);

if($action == "register") {

	if($login == '') $error = "Nie podano loginu";

	else if(!ereg("^[a-z0-9_\-]+$", $login)) $error = "Niepoprawny login";

	else if(strlen($login) <= 2) $error = "Ten login jest za krótki";

	else if(mysql_num_rows(mysql_query("SELECT `login` FROM `users` WHERE `login`='$login' LIMIT 1")) == 1) $error = "Ten login jest zajęty";
		
	else if($pass == '') $error = "Nie podano hasła";
	else if($repass == '') $error = "Nie podano ponownie hasła";
		
	else if($pass != $repass) $error = "Musisz wpisać identyczne hasła";

	else if($mail == '') $error = "Nie podano adresu e-mail";
		
	else if(!eregi("^[a-zA-Z0-9_-]+(\.[a-zA-Z0-9_-]+)*@([a-zA-Z0-9_-]+)(\.[a-zA-Z0-9_-]+)*(\.[a-zA-Z]{2,4})$", $mail))
	$error = "Musisz wpisać prawidłowy adres e-mail";
	
	else {
			mysql_query("INSERT INTO `users` SET `login`='$login', `pass`='".md5($pass)."', `mail`='$mail', `ip`='$ip', `reg_date`=NOW()");
	}
}

else if($action == "login") {

	if(mysql_num_rows(mysql_query("SELECT `login`, `pass` FROM `users` WHERE `login`='$login' AND `pass`='".md5($pass)."' LIMIT 1")) == 0)
	$error = "Niepoprawne dane";
	
	else {
			mysql_query("UPDATE `users` SET `ip`='$ip', `last_login`=NOW() WHERE `login`='$login'");
			$logged = 1;
	}
}

else if($action == "logout") {
	
	$logged = 0;
	header("Location: /");
	
}

else {
?>
<style type="text/css">
td {
	color: #808080;
	width: 150px;
}

span#error_msg {
	font-weight: bold;
	text-align: right;
	float: right;
	color: #ccc;
}
</style>

<form action="#" id="form" method="post" onsubmit="loginRegister(); return false;">
<input type="hidden" id="action" value="login" />
<p>Zaloguj siÄ™ lub
<a href="#" onclick="switchAction()">zarejestruj</a></p>
<br/>

<table>
<tr>
	<td>LOGIN</td>
	<td><input type="text" id="login" maxlength="15" /></td>
</tr>

<tr>
	<td>HASŁO</td>
	<td><input type="password" id="pass" /></td>
</tr>
</table>

<div id="register" style="display: none;">
<table>
<tr>
	<td>POWTÓRZ HASŁO</td>
	<td><input type="password" id="repass" /></td>
</tr>

<tr>
	<td>EMAIL</td>
	<td><input type="text" id="mail" maxlength="40" /></td>
</tr>
</table>
</div>

<p><span id="error_msg"></span><br/>
<br/><input type="submit" onclick="loginRegister(); return false;" value="OK" /></p>
</table>
</form>
<? }
if(isset($error)) print $error;
else print true;
?>

Refactorings

No refactoring yet !

5a00a3a98dcf6f9cd717440fd2b606e5

Eineki

December 13, 2008, December 13, 2008 15:57, permalink

No rating. Login to rate!

I don't think you can simplify the function, it has to perform a lot of simple test.
We can just add a bit of syntax sugar using a case instead of the nested if and elseif in the register action.
For me it is a little more readable.
Maybe you should separate the view (that html part at the end of the code) from the controller part

<?php
ob_start();
include_once "include/config.php";

$login = safe_chars($login);
$pass = safe_chars($pass);

if($action == "register") {
  switch(true) {
    case ($login == ''):
         $error = "Nie podano loginu";
         break;
    case (!ereg("^[a-z0-9_\-]+$", $login)):
         $error = "Niepoprawny login";
         break;
    case (strlen($login) <= 2):
         $error = "Ten login jest za krótki";
         break;
    case (mysql_num_rows(mysql_query("SELECT `login` FROM `users` WHERE `login`='$login' LIMIT 1")) == 1):
         $error = "Ten login jest zajęty";
         break;
    case ($pass == ''):
         $error = "Nie podano hasła";
         break;
    case ($repass == ''):
         $error = "Nie podano ponownie hasła";
         break;
    case ($pass != $repass):
         $error = "Musisz wpisać identyczne hasła";
         break;
    case ($mail == ''):
         $error = "Nie podano adresu e-mail";
         break;
    case (!eregi("^[a-zA-Z0-9_-]+(\.[a-zA-Z0-9_-]+)*@([a-zA-Z0-9_-]+)(\.[a-zA-Z0-9_-]+)*(\.[a-zA-Z]{2,4})$", $mail)):
         $error = "Musisz wpisać prawidłowy adres e-mail";
         break;
    default:
         mysql_query("INSERT INTO `users` SET `login`='$login', `pass`='".md5($pass)."', `mail`='$mail', `ip`='$ip', `reg_date`=NOW()");
}

else if($action == "login") {
   $query = sprintf("UPDATE `users` SET `ip`='%s', `last_login`=NOW() WHERE `login`='%s' AND `pass`='%s'", $ip, $login, md5($pass))
  if(mysql_affected_rows(mysql_query($query)) != 1) // assume there is a single pair login/pass into db
    $error = "Niepoprawne dane";
  else {
    $logged = 1;

} else if($action == "logout") {
  $logged = 0;
  header("Location: /");
}

else {
?>
<style type="text/css">
td {
	color: #808080;
	width: 150px;
}

span#error_msg {
	font-weight: bold;
	text-align: right;
	float: right;
	color: #ccc;
}
</style>

<form action="#" id="form" method="post" onsubmit="loginRegister(); return false;">
<input type="hidden" id="action" value="login" />
<p>Zaloguj siÄ™ lub
<a href="#" onclick="switchAction()">zarejestruj</a></p>
<br/>

<table>
<tr>
	<td>LOGIN</td>
	<td><input type="text" id="login" maxlength="15" /></td>
</tr>

<tr>
	<td>HASŁO</td>
	<td><input type="password" id="pass" /></td>
</tr>
</table>

<div id="register" style="display: none;">
<table>
<tr>
	<td>POWTÓRZ HASŁO</td>
	<td><input type="password" id="repass" /></td>
</tr>

<tr>
	<td>EMAIL</td>
	<td><input type="text" id="mail" maxlength="40" /></td>
</tr>
</table>
</div>

<p><span id="error_msg"></span><br/>
<br/><input type="submit" onclick="loginRegister(); return false;" value="OK" /></p>
</table>
</form>
<? }
if(isset($error)) print $error;
?>

Your refactoring





Format Copy from initial code

or Cancel