CODEWITHSUNDEEP
php
Tim C
Tim C

Reputation: 5714

PHP Calling method inside method

PHP student here Consider the following 2 methods the 1 method checks if a user exists the other registers a user.

public function is_registered($userEmail) 
{
   $this->email = $userEmail;

   $sql = "SELECT * from users WHERE email = :email";

   $stmnt = $db->prepare($sql);
   $stmnt->bindValue(':email', $userEmail);

   $check = $stmnt->execute();

   if($check >= 1) {
      return true;
   }
   else {
      return false;
   }
}


public function register_user($email, $pword, $firstname, $lastname)
{
   $this->email = $email;

   //call method is_registered inside register user class
   $registered = $this->is_registered($email);

   if($registered == true){
      die("USER EXISTS");
   }
   else {
      //continue registration
   }

 }

Although this might not be the best use case example I basically want to know:

  1. Is "normal / good practice" to call a method inside a method. Another use case might be calling a get_email() method inside a login() method or similar method where you need an email address.

  2. What is the difference if I set a property inside a method or just access the passed parameter directly? Example:

Method with set property

public function is_registered($userEmail)
{
   $this->email = userEmail // SET PROPERTY

   $sql = "SELECT * from users WHERE email = :email";
   $stmnt = $db->prepare($sql);
   $stmnt->bindValue(':email', $userEmail);
}

Method with no set property. 

public function is_registered($userEmail) 
{
   //NO SET PROPERTY
   $sql = "SELECT * from users WHERE email = :email";
   $stmnt = $db->prepare($sql);
   $stmnt->bindValue(':email', $userEmail);
}

Hope this makes sense, any help or guidance much appreciated.

Upvotes: 1

Views: 68

Answers (1)

Tommaso Belluzzo
Tommaso Belluzzo

Reputation: 23675

On the point of view of OOP, both approaches are kinda weird. Since your User class doesn't seem to be static, and since the e-mail address is one of the major uniqueness discriminant for authentication, your class should be instantiated with a valorized email property that never changes... either upon database extraction (1) or form filling (2):

$user = new User(..., $auth->email); (1)
$user = new User(..., $_POST['registration_mail']); (2)

On the top of that, a method named is_registered should really not mess with assignments. All it should do is to check whether the current User instance class is defined in your database or not.

In your first approach, the risk is to mess up everything. Let's suppose you have the following user:

$user = new User('John', '[email protected]');

Now, let's see what happens when, by mistake, you pass the wrong email as argument:

$result = $user->is_registered('[email protected]');
echo $user->name; // John
echo $user->email // [email protected]

In your second approach, you should not allow the is_registered to accept any argument since, as I explained you before, that property should be defined upon creation to avoid fatal mistakes. Let's bring back the same user we used in the first example:

$user = new User('John', '[email protected]');

Now, let's see what happens when, by mistake, you pass the wrong email as argument ([email protected] is registered, [email protected] isn't):

$result = $user->is_registered('[email protected]'); // false

So my suggestion is, initialize the property upon the creation of the instance and never mess with it. Alternatively, you could implement a static function into a utility class that, for a given email, performs the desired check. But try to make your OOP design as strict as possible.

Upvotes: 1

Related Questions