1

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.

1
  • @Akintunde007 yeah although not part of question, I use global $db to access $db Commented Jan 4, 2018 at 6:41

1 Answer 1

1

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.

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.