Logowanie (pierwsza klasa) Opcje

[Bent]

Witam,

Dopiero zaczynam przygodę z OOP. Napisałem pierwszą taką klasę:

[PHP] pobierz, plaintext 
class Users extends mysqli
{
  private $login = 'login', $password = 'password', $submit;
 
  private function showLogForm()
  {
    ?>
	<form method="post" action="">
	<table>
	<tr>
	<td>Login: </td><td><input type="text" name="<?php echo $this -> login; ?>" /></td>
	</tr>
	<tr>
	<td>Hasło: </td><td><input type="text" name="<?php echo $this -> password; ?>" /></td>
	</tr>
	<tr>
	<td></td><td><input type="submit" name="submit" value="Zaloguj!" /></td>
	</tr>
	</table>
	</form>
	<?php
  }
 
  private function isSent($submit)
  {
    $this -> submit = $submit;
	if (isset($this -> submit))
	  return true;
	else
	  return false;
  }
 
  private function isFilled()
  {
    foreach ($_POST as $key => $value)
	{
	  if (!isset($key) || empty($value))
	    return false;
	}
	return true;
  }
 
  public function logIn()
  {
    if ($this -> isSent($_POST['submit']))
	  {
	    if ($this -> isFilled())
		  echo 'OK';
		else
		  echo 'Uzupełnij formularz!';
	  }
	else
	  {
	    $this -> showLogForm();
	  }
  }
 
}
 
$user = new Users;
$user -> logIn();
[PHP] pobierz, plaintext 

Mam 2 pytania:

1. W mojej klasie metody wyświetlające formularz i sprawdzające go są typu private, metodą, która to wszystko ogarnia jest metoda logIn(). Czy to dobry pomysł? Czy może lepiej wszystkie te metody ustawić jako public i całość sprawdzać poza klasą po utworzeniu obiektu?

2. Czy dobrym pomysłem jest tworzenie metod wyświetlających formularze czy raczej do takich rzeczy się funkcji nie tworzy?

[MateuszS]

Sam ucze sie OOP ale wg mnie nie powinno byc w zadnej metodzie ani troche HTMLa. Powinienes zrobic osobny plik .phtml z formularzem i go dolaczyc includem. No i nie wiem po co przypisales atrybutowi name wartosc ze zmiennej, zmienna, czyli jak sama nazwa wskazuje, moze sie zmieniac (jej wart.), a nie wiem w jakim celu zmieniac name inputa, a wartosci POSTa mozna przeslac osobnymi metodami typu setLogin i setPassword

Ten post edytował MateuszScirka 26.02.2010, 17:48:52

[Bent]

No i nie wiem po co przypisales atrybutowi name wartosc ze zmiennej, zmienna, czyli jak sama nazwa wskazuje, moze sie zmieniac (jej wart.), a nie wiem w jakim celu zmieniac name inputa, a wartosci POSTa mozna przeslac osobnymi metodami typu setLogin i setPassword

Nie bardzo wiem, o co Ci chodzi. Jeśli o to:

Kod

<td>Login: </td><td><input type="text" name="<?php echo $this -> login; ?>" /></td>
</tr>
<tr>
<td>Hasło: </td><td><input type="text" name="<?php echo $this -> password; ?>" /></td>

Chodziło mi o to, żeby skrypt był bardziej uniwersalny.

[MateuszS]

No ok, ale po co ktos by chcial zmieniac name pol? Moze jest w tym jakis cel, ale ja go nie znam i nie rozumiem

Ten post edytował MateuszScirka 26.02.2010, 19:14:34

[darko]

Moim zdaniem klasa odpowiedzialna za logowanie powinna zawierać tylko logikę związaną z autentykacją użytkownika. U Ciebie wydaje się, że wszystko jest pomieszane ze sobą:
- masz klasę o nazwie Users - nazwa mylnie sugeruje, że klasa jest odpowiedzialna za zarządzanie jakąś kolekcją użytkowników
- ta klasa - oprócz wyplucia formularza - kompletnie nic nie robi
- walidacja formularza tylko na zasadzie czy wartości z pól formularza: login i pass są nie puste - to za mało (sprawdzaj długość loginu i hasła)
- metoda showLogForm jest kompletnie nieelastyczna, narzucasz z góry nazewnictwo pól formularza, nie ma możliwości zmiany atrybutu action, pole dla hasła powinno mieć typ password
- ciało metod isSent i isFilled można byłoby zastąpić dwiema liniami kodu
-
Rozumiem, że klasę będziesz rozbudowywał (jako, że dziedziczysz z klasy mysqli)
Ad.1
Sam mechanizm działania klasy jak najbardziej powinien być ukryty poprzez zastosowanie hermetyzacji, jednak zastanowiłbym się nad rozluźnieniem dostępu do metod z private do protected, dzięki temu można będzie je odziedziczyć w klasach potomnych (w przypadku private - nie można). Metody o dostępie prywatnym powinny wykonywać kod, którego nie chcemy udostępniać na zewnątrz, ani nie chcemy, aby klasy wyprowadzane z naszej klasy dziedziczyły określone zachowanie (i/lub właściwości).

Ad. 2
To naprawdę zależy od wielu czynników, ale generalnie nie powinno się umieszczać htmla w klasach.

// edit
chyba zupełnie zapomniałeś o jednej z podstawowych zasad bezpieczeństwa - nigdy nie ufać danym pochodzącym od użytkowników.

//edit2
Na razie nie ma jeszcze czego oceniać, dodaj konkretną implementację logowania + zabezpieczenia przed potencjalnymi zagrożeniami.

Ten post edytował darko 2.03.2010, 11:27:35