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?

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

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



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

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

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.