[PHP] pobierz, plaintext 
<?php
class rejestracja {
	public $login;
	public $uwagi = '';
	private $mail;
	private $pass;
	private $pass2;
 
	public function __construct($login, $mail, $pass, $pass2) {
		$this -> login = $login;
		$this -> mail = $mail;
		$this -> pass = $pass;
		$this -> pass2 = $pass2;
    }
 
	public function SprawdzLogin() {
		if(strlen($this->login) > 20) {
			$this -> uwagi = 'Login jest za długi. <a href="rejestracja.php">Spróbuj ponownie.</a>';
		} elseif(!preg_match('/^[a-zA-Z0-9_\-]+$/', $this->login)) {
			$this -> uwagi = 'Login posiada błędne znaki! <a href="rejestracja.php">Spróbuj ponownie.</a>';
		} elseif(1 > 0) {
			$this -> pre1 = baza::instance() -> prepare('SELECT id FROM users WHERE login = ? COLLATE utf8_bin limit 1');
			$this -> pre1 -> execute(array($this->login));
			if(($this -> pre1 -> rowCount()) > 0) { // jeśli login istnieje
				$this -> uwagi = 'Obywatel o takim loginie już istnieje. <a href="rejestracja.php">Spróbuj ponownie.</a>';
			}
		}
	}
 
	public function SprawdzMail() {
		if(strlen($this -> mail) > 45) {
			$this -> uwagi = 'E-mail jest za długi. <a href="rejestracja.php">Spróbuj ponownie.</a>';
		} elseif(!preg_match('/^[_\.0-9a-zA-Z-]+@([0-9a-zA-Z][0-9a-zA-Z-]+\.)+[a-zA-Z]{2,6}$/i', $this -> mail)) {
			$this -> uwagi = 'Wpisany adres e-mail jest niepoprawny! <a href="rejestracja.php">Spróbuj ponownie.</a>';
		} elseif(1 > 0) {
			$this -> pre2 = baza::instance() -> prepare('SELECT id FROM users WHERE mail = ?');
			$this -> pre2 -> execute(array($this -> mail));
			if(($this -> pre2 -> rowCount()) > 0) { // jeśli mail istnieje
				$this -> uwagi = 'Obywatel o takim adresie e-mail już istnieje. <a href="rejestracja.php">Spróbuj ponownie.</a>';
			}
		}
	}
 
	public function SprawdzHaslo() {
		if(strlen($this -> pass) > 5) {
			$this -> uwagi = 'Hasło jest za krótkie. <a href="rejestracja.php">Spróbuj ponownie.</a>';
		} elseif(!$this -> pass === $this -> pass2) {
			$this -> uwagi = 'Hasła się różnią. <a href="rejestracja.php">Spróbuj ponownie.</a>';
		}
	}
 
	public function Rejestruj() {
		$this -> pass = md5($this -> pass); // koduję hasło
		$this -> data = time(); // zapisuję datę rejestracji
		$this -> pre3 = baza::instance() -> prepare('INSERT INTO users (login, haslo, mail, data_rej) VALUES (:login, :pass, :mail, :data)');
		$this -> pre3 -> execute(array(':login' => $this -> login, ':pass' => $this -> pass, ':mail' => $this -> mail, ':data' => $this -> data));
		$this -> uwagi = 'Obywatelu <b>'.$this -> login.'</b>! Zostałeś przyjęty do państwa! <a href="logowanie.php">Zaloguj się!</a>';
	}
}
?>
[PHP] pobierz, plaintext 

Kawałek z pliku rejestracji (formularz etc):

[PHP] pobierz, plaintext 
if (isset($_POST['rejestruj'])) { // czy kliknął SUBMIT?
        if ($_POST['login'] != '' && $_POST['mail'] != '' && $_POST['pass'] != '' && $_POST['pass2'] != '') { // czy pola nie są puste?
            require_once 'klasy/rejestracja.php';
            $rejestracja = new rejestracja($_POST['login'], $_POST['mail'], $_POST['pass'], $_POST['pass2']);
            $rejestracja -> SprawdzLogin();
            if($rejestracja -> uwagi != '') {
                echo $rejestracja -> uwagi;
            } elseif($rejestracja -> uwagi = '') {
                $zapytania++;
                $rejestracja -> SprawdzMail();
                echo $rejestracja -> uwagi;
            } elseif($rejestracja -> uwagi != '') {
                echo $rejestracja -> uwagi;
            } elseif($rejestracja -> uwagi = '') {
                $zapytania++;
                $rejestracja -> SprawdzHaslo();
            } elseif($rejestracja -> uwagi != '') {
                echo $rejestracja -> uwagi;
            } else {
                $rejestracja -> Rejestruj();
                $zapytania++;
                echo $rejestracja -> uwagi;
            }
        }
        else { // a jak się czegoś zapomniało
            echo 'Brakuje danych! <a href="rejestracja.php">Spróbuj ponownie.</a>'; // to niech zacznie od nowa
        }
    }
    else {
    wyswietlanie formularza
[PHP] pobierz, plaintext 

Pytania:
1. Co robię źle?
2. Da się to jakoś napisać bardziej optymalnie?

Przerzuć część walidacji na przeglądarkę np: długości wpisanych stringów.
Utwórz jedno połączenie do bazy danych w Classie ( stwórz własną klasę albo korzystaj z gotowej np. z PDO http://php.net/manual/en/book.pdo.php ) zamiast odnosić się do singletona ( baza::instance() ).
Dlaczego odnosisz się do tych zmiennych jako globalnych skoro niemi nie są ? => pre1 ,pre2 , pre3 ,data,ile2 ,ile1

po co tworzysz dodatkowe zmienne?

[PHP] pobierz, plaintext 
$this -> ile1 = $this -> pre1 -> rowCount();
if($this -> ile1 > 0)
[PHP] pobierz, plaintext 

zamiast

[PHP] pobierz, plaintext 
if( ( $this -> pre1 -> rowCount() ) > 0)
[PHP] pobierz, plaintext 

użyj else zamiast

[PHP] pobierz, plaintext 
elseif(1 > 0)
[PHP] pobierz, plaintext 

podziel klase na validatory i część wykonawczą,jak jakiś validator wywali to dopiero na końcu wyświetl zmienną uwagi

[PHP] pobierz, plaintext 
if( $rejestracja -> SprawdzLogin() && $rejestracja -> SprawdzMail() && $rejestracja -> SprawdzHaslo() ){
     $rejestracja -> Rejestruj();
}else{
   echo $rejestracja ->uwagi;
}
[PHP] pobierz, plaintext 

Pole login i mail na maxlength taki jak w klasie, pole mail jest type="email" (dla nowszych przeglądarek, starsze widzą to jako text). Dodatkowo dla bezpieczeństwa usera dodałem siłę hasła (skrypty JS nie mój).


Ja korzystam z PDO. klasa Baza służy do połączenia z MySQL z wykorzystaniem PDO. Kod klasy: http://wklej.to/8SPv8 hash: 123


Czyli mam zrobić private $pre1; private $pre2; dla wszystkich takich zmiennych?


Poprawione



Poprawione


Jak mam klasę przebudować?


----
Uaktualniony kod znajduje się w pierwszym poście.

Zminiłem Twoją klasę na coś takiego

[PHP] pobierz, plaintext 
<?php
class rejestracja {
	public $login;
	public $uwagi = '';
	private $mail;
	private $pass;
	private $dbConnect = null;
 
	public function __construct( $login, $mail, $pass ) {
		$this -> login = $login;
		$this -> mail = $mail;
		$this -> pass = $pass;
		$this->dbConnect = new Instancja_Klasy_Laczaca_Z_DB_Przez_PDO(); // pkt 1
 
	}
 
	public function SprawdzLogin() {
		if( strlen($this->login) > 20 ){
			$this -> uwagi = 'Login jest za długi. <a href="rejestracja.php">Spróbuj ponownie.</a>';
			return false; //pkt 2
		}
		if( !preg_match( '/^[a-zA-Z0-9_\-]+$/', $this->login ) ){
			$this -> uwagi = 'Login posiada błędne znaki! <a href="rejestracja.php">Spróbuj ponownie.</a>';
			return false; //pkt 2
		}
		$this->dbConnect->prepare('SELECT id FROM users WHERE login = ? COLLATE utf8_bin limit 1');
		$this->dbConnect->execute( array( $this->login ) );
		if( $this->dbConnect->rowCount() > 0 ) { // jeśli login istnieje
			$this -> uwagi = 'Obywatel o takim loginie już istnieje. <a href="rejestracja.php">Spróbuj ponownie.</a>';
			return false; //pkt 2
		}else{
			return true; //pkt 2
		}
	}
 
	public function SprawdzMail() {
		if(strlen( $this -> mail ) > 45 ) {
			$this -> uwagi = 'E-mail jest za długi. <a href="rejestracja.php">Spróbuj ponownie.</a>';
			return false; //pkt 2
		}
		if(!preg_match('/^[_\.0-9a-zA-Z-]+@([0-9a-zA-Z][0-9a-zA-Z-]+\.)+[a-zA-Z]{2,6}$/i', $this -> mail)) {
			$this -> uwagi = 'Wpisany adres e-mail jest niepoprawny! <a href="rejestracja.php">Spróbuj ponownie.</a>';
			return false; //pkt 2
		}
		$this->dbConnect->prepare('SELECT id FROM users WHERE mail = ?');
		$this->dbConnect->execute(array($this -> mail));
		if( ( $this->dbConnect->rowCount() ) > 0 ) { // jeśli mail istnieje
			$this -> uwagi = 'Obywatel o takim adresie e-mail już istnieje. <a href="rejestracja.php">Spróbuj ponownie.</a>';
			return true; //pkt 2
		}else{
			return true; //pkt 2
		}
	}
 
	public function SprawdzHaslo( $sPasswordConfirm ) {
		if(strlen($this -> pass) > 5) {
			$this -> uwagi = 'Hasło jest za krótkie. <a href="rejestracja.php">Spróbuj ponownie.</a>';
			return false; //pkt 2
		}
		if( ! $this->pass == $sPasswordConfirm ) {
			$this->uwagi = 'Hasła się różnią. <a href="rejestracja.php">Spróbuj ponownie.</a>';
			return false; //pkt 2
		}
		return true; //pkt 2
	}
 
	public function Rejestruj() {
		$this -> pass = md5($this -> pass."super tajny string znany tylko tobie zarejestrowany gdzieś w kodzie"); // koduję hasło // pkt 3
		$this->dbConnect-> prepare( 'INSERT INTO users (login, haslo, mail, data_rej) VALUES (:login, :pass, :mail, :data)');
		$this->dbConnect-> execute( array( ':login' => $this -> login, ':pass' => $this -> pass, ':mail' => $this -> mail, ':data' => 'NOW()' ) );
		$this -> uwagi = 'Obywatelu <b>'.$this -> login.'</b>! Zostałeś przyjęty do państwa! <a href="logowanie.php">Zaloguj się!</a>';
	}
}
[PHP] pobierz, plaintext 

a samo wywołanie na coś takiego :

[PHP] pobierz, plaintext 
<?php
if ( isset( $_POST['rejestruj'] ) ) { // czy kliknął SUBMIT?
	if ( $_POST['login'] != '' && $_POST['mail'] != '' && $_POST['pass'] != '' && $_POST['pass2'] != '') { // czy pola nie są puste?
		require_once 'klasy/rejestracja.php';
		$rejestracja = new rejestracja( $_POST['login'], $_POST['mail'], $_POST['pass'] );
		if( $rejestracja -> SprawdzLogin() && $rejestracja -> SprawdzMail()	&& $rejestracja -> SprawdzHaslo( $_POST['pass2'] ) ){
			$rejestracja -> Rejestruj();
		}
		echo $rejestracja -> uwagi;
	}else { // a jak się czegoś zapomniało
		echo 'Brakuje danych! <a href="rejestracja.php">Spróbuj ponownie.</a>'; // to niech zacznie od nowa
	}
}else {
	wyswietlanie formularza
?>
[PHP] pobierz, plaintext 

objaśnienie :

1) w Twoim kodzie wywoływałeś 3 razy połączenie z Bazą danych , w taki sposób robisz to tylko raz.Nie wiem czy masz autoloader więc jak to wywołać musisz sam już ogarnąć.Klasa baza którą traktowałeś jako singleton, jest dla mnie nie potrzebna. Stwórz sobie klasę tworzącą połączenie z twoimi danymi i dziedzicząca po PDO
2) zwracasz czy stan validacji , nie musisz potem sprawdzać czy zmienna uwagi jest pusta
3)Przedłużaj string na hasło, często ludzi wpisują bardzo popularne wyrazy, wystarczy wtedy md5 wkleić chociaż by w google i już się ma hasło.Przy przedłużeniu stringu jest to nie możliwe.Poza tym zwiększa bezpieczeństwo
4) wywołanie klasy krótsze, mniej if'ów, bardziej czytelne
5)mniejsza ilość zmiennych = mniej miejsca w pamięci

stosuj się do reguł
KISS - http://pl.wikipedia.org/wiki/Kiss
DRY - http://pl.wikipedia.org/wiki/DRY