OOP - System Rezerwcacji, Sprwadzenie kodu Opcje

[JakubBab]

Czesc,

Pisze (dla praktyki) system wypozyczania ksiazek. Narazie skonczylem system rejestracji uzytkownikow. Jest to pierwszy projekt w OOP jaki robie wiec napewno sa bledy. Prosba o sprwadzenie formy i wskazowki jak i co polepszyc . Tak towyglada w plikach:

projekt/config/config.php
projekt/config/database.php

projekt/class/user.class.php
projekt/class/userException.class.php

projekt/controller/register.control.php

projekt/register.php

A tu kod:

config/config.php

[PHP] pobierz, plaintext 
<?php
 
$host = substr($_SERVER['HTTP_HOST'],0,5);
 
if(in_array($host, array('local','127.0','192.1'))){
	$local=TRUE;
}else
	{
			$local= FALSE;
	}
 
//determine location of files and the URL of the site: 
//allow for development on different servers. 
 
if($local){
	$debug=TRUE;
//define the constants: 
	define('BASE_URI','c:\xampp\htdocs\library');
	define('BASE_URL','http://localhost/library/');
	define('DB','c:\xampp\htdocs\library\\config\database.php');
 
} // if there was a live server then the condition would have included live server defines
 
if(!isset($debug)){
	$debug=FALSE;
}
 
// Custom defined error handler - no need to define it as of yet. 
 
?>
[PHP] pobierz, plaintext 

class/user.class.php

[PHP] pobierz, plaintext 
/*
 * user.class.php 
 * Adds user : First Name, Second Name, Email, ID - auto increment
 * Checks if email is already in the db. - Unique email field
 * Adds user into the detabase 
 */
 
class user{
 
 
	//db 
	private $conn;
 
	public $name,$surname,$email;
	public $password;
	public $_message; // for assigning errors
 
	function __construct($db){
	$this->conn=$db;
	}
 
	/*
	 *Name:  not numeric, less then 10 characters
	 */
 
	function checkName($name){
		$this->name=$name;
		if(!is_numeric($this->name)
		&& strlen($this->name) < 10){
			return $this->name;
		}
		Throw New userException($this->_message,0);
	}
	/*
	 *  Surname: not numeric, less then 10 characters
	*/
 
	function checkSurname($surname){
		$this->Surname=$surname;
		if(!is_numeric($this->Surname)
		&& strlen($this->Surname) < 10){
			return $this->urname;
		}
		Throw New userException($this->_message,1);
	}
 
	/*
	 * The password's first character must be a letter, it must contain at least 4 
	 * characters and no more than 15 characters and no characters other than letters, 
	 * numbers and the underscore may be used
	 */
 
	function checkPassword($password){
		$this->password=$password;
		if(strlen($this->password) < 10
		 && preg_match('/^[a-zA-Z]\w{3,14}$/',$this->password)){
		 	return $this->password;
		 }else{
		 	Throw New userException($this->_message,2);
		 }
	}
 
 
/*
 * Password hashing
 */
 
	function setPassword($password){
		$this->password=sha1($password);
		return $password;
	}
 
 
 
	function create(){	
	$query= "insert into users SET name=?, Surname=?, email=?, password=?";
 
 
	 $stmt = $this->conn->prepare($query);
 
	$stmt->bindParam(1,$this->name);
	$stmt->bindParam(2,$this->Surname);
	$stmt->bindParam(3,$this->email);
	$stmt->bindParam(4,$this->password);
 
	if($stmt->execute()){
		return TRUE;
	}else{
		Throw New Exception('email');
	}
 
}// end of create()
 
 
} // end of main if
 
 
[PHP] pobierz, plaintext 

class/exception.class.php

[PHP] pobierz, plaintext 
class userException extends Exception{
 
	public function getDetails(){
 
		switch ($this->code){
 
			case 0:
				return '</br>Name. ';
				break;
			case 1:
				return '</br>Surname</br>Details given do not meet required criteria';
				break;
			case 2:
				return 'Password must contain 1 character, special sign, upper and lower case letters';
			break;
			case 3:
				return 'There is something wrong with the email';
			default:
				return 'no Further information is available';
				break;	
 
 
		}
	}
 
}
 
[PHP] pobierz, plaintext 

controller/register.control.php

[PHP] pobierz, plaintext 
if(isset($_POST['send'])){
if($_SERVER['REQUEST_METHOD']=='POST'
&& isset($_POST)
&& !empty($_POST)
&& filter_var($_POST['em'],FILTER_VALIDATE_EMAIL)
){
 
include_once('./class/user.class.php');
include_once('./config/database.php');
require_once('./class/exception.class.php');
 
$database = new Database;
$db=$database->getConnection();
$user= new user($db);
 
 
try{
$user ->name =$_POST['fn'];
$user->checkName($user->name);
 
$user ->Surname = $_POST['sn'];
$user->checkSurname($user->Surname);
 
$user ->password= $_POST['ps'];
$user->checkPassword($user->password);
$user ->setPassword($user->password);
 
 
$user ->email = $_POST['em'];
 
 
 
 	if($user->create()){
 
		Header('Location:./includes/good.php');
	}  
}catch(userException $e){
	echo "<div class=\"alert alert-info\" alert-dismissable\" id=\"err_blank\">";
            echo "<button type=\"button\" class=\"close\" data-dismiss=\"alert\" aria-hidden=\"true\">&times;</button>";
            echo "Please check the following: {$e->getDetails()}";
        echo "</div>";
}
 
 
} // end of secondary if
 
} // end of main if
[PHP] pobierz, plaintext 

view/register.php

[PHP] pobierz, plaintext 
<!DOCTYPE html>
<html lang="en">
  <head>
    <!-- Required meta tags always come first -->
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
    <meta http-equiv="x-ua-compatible" content="ie=edge">
 
    <!-- Bootstrap CSS -->
    <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0-alpha.2/css/bootstrap.min.css" integrity="sha384-y3tfxAZXuh4HwSYylfB+J125MxIs6mR5FOHamPBG064zB+AFeWH94NdvaCBm8qnd" crossorigin="anonymous">
    <link rel="stylesheet" href= "css/bootstrap-theme.min.css">
  </head>
  <body>
    <h1>Registration Form</h1>
 
    <!-- jQuery first, then Bootstrap JS. -->
    <script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.4/jquery.min.js"></script>
    <script src="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0-alpha.2/js/bootstrap.min.js" integrity="sha384-vZ2WRJMwsjRMW/8U7i6PWi6AlO1L79snBrmgiDpgIWJ82z8eA5lenwvxbMV1PAh7" crossorigin="anonymous"></script>
  </body>
</html>
 
<!--			header -->
 
<?php 
 
require_once('controller/register.control.php');
?>
 
<div id="wrapper">
<div class="col-xs-3">
 
<form class="form-block" action="" method="post">
 
  <div class="form-group">
    <label for="exampleInputName2">Name</label>
    <input type="text" class="form-control"  name="fn" id="exampleInputName2" placeholder="First Name">
  </div>
  <div class="form-group">
    <label for="exampleInputName2">Surname</label>
    <input type="text" class="form-control" name="sn" id="exampleInputName2" placeholder="Surname">
  </div>
   <div class="form-group">
    <label for="exampleInputName2">Password</label>
    <input type="text" class="form-control" name="ps" id="exampleInputName2" placeholder="Password">
  </div>
  <div class="form-group">
    <label for="exampleInputEmail2">Email</label>
    <input type="email" class="form-control" name="em" id="exampleInputEmail2" placeholder="email adress">
  </div>
  <button type="submit" name="send" class="btn btn-primary">Register</button>
</form>
</div>
</div>
[PHP] pobierz, plaintext 

[viking]

Niepotrzebnie nazywasz pliki ...class.php. Tak się robiło kilka lat temu. Poczytaj o PSR-4 + autoloader np. composera.
Stosuj modyfikator widoczności dla metod.
Klasa User to raczej jakiś service w tym wypadku. Jeśli rozdzielisz to do encji będziesz mógł łatwo wymieniać dane z tablicy POST do obiektu.
Stosuj rzutownie typów np. public function __construct(\PDO $db)
Walidacja jest w kilku miejscach.
Dlaczego hasło ma takie restrykcje? Wrzucaj je do password_hash/validate.
Zdecyduj się na jednolite nazewnictwo bo jest raz name a później Surname.

[com]

Wszystkie throw, new, header małymi literami, tak samo jak true, false.
Zadbaj o odpowiednie formatowanie, nwm jakiego edytora używasz ale przeważnie jest tak że zaznaczasz wszystko i alt + i (ctrl+alt+i).
Nie zapominaj o hermetyzacji, pola nie powinny być publiczne.

Jak popracujesz nad formatowaniem to nie będziesz potrzebował komentarzy końca metod itp

[JakubBab]

Ok. Dzieki.

Poczytalem i dochodze do wniosku, ze... jeszcze raz to napisze.

Klasa - db - Singleton
Klasa action dla db - z delete, select itp.
I walidacja w jednym miejscu

Az takich restrykcji nie ma na haslo, bylo wieksze ale zmienilem na cos przyzwoitego. W switchu jest opis starego regex-u.

Pozmienialem widocznosc:

[PHP] pobierz, plaintext 
private $conn;
 
	private $name;
	private $surname, $email;
	private $password;
	private $_message;
[PHP] pobierz, plaintext 

Co do edytora - Aptana - moze komus sie przyda w przyszlosci - (ctrl+shift+F).

Niestety, reszte musze poprawic dopiero jutro. Bede update-owac.

Ten post edytował JakubBab 1.02.2016, 16:48:42

[com]

pomyśl nad depedency injection, zamiast antywzorca Singleton

undescore w nazwie raczej zbędne

[JakubBab]

pomyśl nad depedency injection, zamiast antywzorca Singleton

undescore w nazwie raczej zbędne

Czytam o DI i patrze na moj kod - czy przypadkiem nie ma tam dependency injection?

class db - odziellnie

class user

[PHP] pobierz, plaintext 
class user {
 
	//db
	// for assigning errors
 
	function __construct(PDO $db) {
		$this -> conn = $db;
	}
[PHP] pobierz, plaintext 

no i register.class

[PHP] pobierz, plaintext 
$database = new Database;
		$db = $database -> getConnection();
		$user = new user($db);
[PHP] pobierz, plaintext 

Widze, ze zle jest to rozlozone plikowo. Pisze od nowa. Sek jest w tym, czy jest to DI jesli nie, jak to ugryzc?

[com]

Ja nie powiedziałem że nie masz, powiedziałem żeby Singleton zastąpić DI