Skip to main content
added 48 characters in body
Source Link
KIKO Software
  • 6.1k
  • 15
  • 24

Your code seems to be dealing with a login form. TheYour class therefore should be called something like: ProcessLoginFormLoginFormInput. A good login can result in an User Class, but the processing of user input should not be done in the User Class itself.

https://www.guru99.com/object-oriented-programming.html

https://code-boxx.com/simple-php-mvc-example

Your code seems to be dealing with a login form. The class therefore should be called something like: ProcessLoginForm. A good login can result in an User Class, but the processing of user input should not be done in the User Class itself.

https://www.guru99.com/object-oriented-programming.html

Your code seems to be dealing with a login form. Your class therefore should be called something like: LoginFormInput. A good login can result in an User Class, but the processing of user input should not be done in the User Class itself.

https://www.guru99.com/object-oriented-programming.html

https://code-boxx.com/simple-php-mvc-example

Source Link
KIKO Software
  • 6.1k
  • 15
  • 24

I can understand why no one has yet answered your question. The basic problem is that your User Class is not really an User Class. All it does is process user input, and that's not something an User Class should do. An User Class should deal with an user, nothing else. Things like:

  • Changing user name and/or password.
  • Verifying passwords.
  • Keep information about an user.
  • Keeping user information in the database up to date.
  • permissions.

Basically anything that has to do with the user once the user is known. The basic structure of an user class therefore should be:

class User {

    public function __construct($userId) {
    }

}

Your code seems to be dealing with a login form. The class therefore should be called something like: ProcessLoginForm. A good login can result in an User Class, but the processing of user input should not be done in the User Class itself.

A good guide into designing a class, like this, is the Model–view–controller (MVC) pattern. It clearly separates input, internal logic, and output.

Talking about user input processing. This should be done with care. How you deal with user input largely determines how secure your code is. Dumping all of $_POST directly into a class can be safe, if that class was intentionally designed to deal with it, but in most cases you want to filter user input at the top of a PHP script. In your case you could do something like:

<?php

$input["login"]    = filter_input(INPUT_POST, "user_login", FILTER_SANITIZE_STRING);
$input["password"] = filter_input(INPUT_POST, "user_pwd",   FILTER_UNSAFE_RAW);
$input["email"]    = filter_input(INPUT_POST, "user_email", FILTER_SANITIZE_EMAIL);
$input["name"]     = filter_input(INPUT_POST, "user_name",  FILTER_SANITIZE_STRING);

Only four inputs are now accepted, nothing else, and most of them are sanitized before they go into $input. Notice that I intentionally did not filter the password. This way any user password is possible. This filtering therefore doesn't make the content of $input safe, it is still user input and should be treated with care.

I normally process each form in its own PHP script, specifically written for that form. The code above would be the start, and most of your User Class would form the rest of the code. The end result could be a valid $userId which can be stored in the session, and can be used to create an User object.

One other thing in your code, that goes against everything I have learned, is the use of call_user_func_array() in verify_fields(). Just don't do that. It is bad for a lot of reasons, like maintainability and testability, but mostly because it is simply difficult to understand what it exactly does.

Your error processing is also unclear. You use both exceptions and error responses. It is one or the other, not both.

Also pay attention to the names you choose. I've already discussed the class name, but there's more. The obvious typo in vertify_fields(), a dataParser() method that doesn't do any parsing, and I still wonder what the differences between the get() and getData() methods are. I can't tell from the name.

A class should be used to encapsulate the inner workings of whatever the class is dealing with. It should abstract away from the details and give you a clean interface. Your class doesn't do this.

In the end I have to conclude that your class is probably syntactically correct, but that's about it. It seems like you haven't fully understood the reasons why we use OOP.

There are lot's of tutorials on the internet. Some of them only explain the syntax, others explain how objects relate to each other, but there are very few that correctly explain how to use them effectively. I can't find one I really like. However, if you take them all together you will get the idea:

https://www.guru99.com/object-oriented-programming.html

https://www.valuebound.com/resources/blog/object-oriented-programming-concepts-php-part-1

https://www.studytonight.com/php/php-object-oriented-programming