[PHP] Pierwszy skrypt na klasach - dobry ? Opcje

[Dominator]

Powoli przerzucam się z programowania strukturalnego na obiektowe, więc napisałem skrypt.
Dobrze napisałem ? Co zmienić/poprawić ?

[PHP] pobierz, plaintext 
<?php
class User
{
	public $_login;
	public $_password;
	public function Login($_login, $_password)
	{
		$this->login = $_login;
		$this->password = $_password;
		if($this->login == "123")
		{
			if($this->password == "456")
			{
				return "Logged";
			}
			else
			{
				return "Bad password";
			}
		}
		else
		{
			return "Bad username";
		}
	}
}
 
if($_GET['username'] && $_GET['password'])
{
	$user = new User;
	echo $user->Login($_GET['username'], $_GET['password']);
}
[PHP] pobierz, plaintext 

Ten post edytował Dominator 8.09.2012, 22:32:18

[SmokAnalog]

Pomijając wątpliwą logikę tego skryptu, sama struktura obiektowa jest w miarę poprawna poza jednym szczegółem. Mianowicie, deklarujesz w klasie pole (zmienne) $_login i $_password, a wewnątrz metody login() przypisujesz wartości do pól $login i $password. Błędu PHP nie wyrzuci, bo nie wymaga deklaracji pól, ale "niesmak pozostaje" (IMG:style_emoticons/default/smile.gif)

Pomysł ze zwracaniem łańcucha znaków w metodzie login() jest dosyć słaby, ale domyślam się, że dopiero zacząłeś przygodę z obiektowością i - sądząc po jakości kodu - z samym PHP, więc zapewne sam do tego dojdziesz.

[Szymciosek]

Można byłoby trochę krócej, a po drugie nie przekazuje się takich danych jak login/password w $_GET, jak już to $_POST, ale do tego sam dojdziesz jak poczytasz, dalej... nie wyświetla się informacji o tym, że sam login jest zły, albo jeśli login podam dobry, to dostaję informację o tym, że hasło złe...

coś w ten deseń jak już:

[PHP] pobierz, plaintext 
if ($login == 'Szymek' && $password = '123')
{
//zaloguj
} else {
//wyswietl informacje: Błędny login / hasło
}
[PHP] pobierz, plaintext 

[!*!]

Ujdzie + to co napisali wyżej, dodam od siebie żebyś zamienił w edytorze tabulacje na spacje, oraz poczytał o PSR-0/1/2 i się tych zasad trzymał.

[rivos]

[PHP] pobierz, plaintext 
<?php
class User
{
	public $_login;
	public $_password;
	public function __construct($_login, $_password)
	{
		if($_login == "123") {
			if($_password === "456") {
				return "Logged";
				$this->login = $_login;
				$this->password = $_password;
			}else {
				return "Bad password";
			}
		}else {
			return "Bad username";
		}
	}
}
 
if($_GET['username'] && $_GET['password'])
{
	echo $user = new User($_GET['username'], $_GET['password']);
}
[PHP] pobierz, plaintext 

wydaje mi się, że będzie Ci tak wygodniej (IMG:style_emoticons/default/biggrin.gif)

Ten post edytował rivos 9.09.2012, 09:39:49

[Szymciosek]

Jaki jest sens powiadamiania użytkownika, że tylko hasło jest złe ?

Będzie w bazie użytkownik XXX, ja się o tym dowiem, bo podając dobry login, a złe hasło dostanę taką informację i później tylko zgadywać hasło...

[-mas-]

no i przydaloby sie zmienne $_login i $_password ustawic jako private. Nie wiem co to jest $this->login skoro w klasie masz zadeklarowane zmienne $_login i $_password

podając w konstruktorze zmienna $_login musisz ja przypisac do zmiennej w klasie w twoim przypadku o takiej samej nazwie
$this->_login=$_login;