7
\$\begingroup\$

I've recently picked up OOP in Java and I'm trying to use it for my PHP user system.

I've come up with the following generic.class. On my main site, I'll simply put include('generic.class.php'); at the top of the file before my content.

<?php
include_once('connect.class.php');

class User {

    private $id;
    private $username;
    private $email;

    public function __construct(){
        $this->id = $_SESSION['id'];
        $this->username = $_SESSION['username'];
        $this->email = $_SESSION['email'];
    }

}

class Generic {

    private $maintenance = false;

    public function __construct() {     
        // Start session
        session_start();

        // Check maintenance status
        if($this->maintenance){
            include('maintenance.php');
            die();
        }

        if(user_logged_in()){
            $user = new User();
        }
    }
}

function user_logged_in(){
    if (isset($_SESSION['username'], $_SESSION['id'])){
        return true;
    } else {
        return false;
    }
}

$generic = new Generic();
$conn = new Conn();
?>

Echoing $user->username doesn't seem to work. Or is there no need to use a User class at all? (most PHP user systems I see don't have a User class)

Is this the right approach to using OOP and classes with a user system? If not, how should my classes be like?

\$\endgroup\$
1
\$\begingroup\$

Couple of points:

  • Utilize Dependency Injection for your constructor to loosely couple it's dependencies.
  • In OOP, functions are not used that often, but methods. user_logged_in() should reside in some sort of User authentication handler (Single Responsibility Principle). Where you could do something like $usrAuthService->isLoggedIn().
  • Give your classes meaningful names. Generic and Conn are really bad names. What is a "Generic" class? What is "Conn"? Connection? Connection of what? In a month when you come back to visit your code, you will not understand at a glance what is happening.
  • A constructor's primary purpose is to initialize the object.

$user->username doesn't work because your class properties are private.

You must realize that there are many different design patterns out there that programmers may adapt for their systems. User systems that have entity classes like User are more domain oriented (DDD).

\$\endgroup\$
0
\$\begingroup\$

As others mentioned the code is not completly OOP.

A real OOP application does not contain functions but classes and methods. Well, except of the file that includes the class that initialize the application and the autoloader function - AFAIK autoloader_register function expects a function only.

The autoloading part is missing as well which is part of dependency injection. I recommend to get into it.

In the constructor of class User you do not validate the email etc. that are stored in the session. They should be a) passed as arguments to the constructor and b) validated. E.g. in the protected void setEmail($email) {} method.

Another point is the php closing. To avoid errors never close php. It's hard to figure out a space or another character after php-closing.

\$\endgroup\$
0
\$\begingroup\$

No, I'm afraid this is not good OOP.

The User constructor takes no arguments, so it's unclear where its fields will come from.

It would be better to either make its parameters explicit, by passing them as constructor arguments, or to create a factory method whose name describes where the user fields will come from, for example UserFactory.createFromSession.

The Generic class is poor in many ways:

  • The word "Generic" doesn't mean anything (naturally). A class should have a good name that describes its purpose. When it's not clear how to name a class, it indicates a problem with the design.

  • The purpose of a constructor is to create an object. This constructor does something else. In fact it's just a utility method that shouldn't be in a class at all.

user_logged_in is not a great name. It would be better to move this into a private method of UserFactory, and rename it to is_logged_in (the "user" is implied by the class). The factory method could return two kinds of users:

  1. Regular user, logged in, authenticated
  2. Anonymous user, not logged in

You could have different classes to represent these two types, sharing the same interface, and having a method like is_authenticated_user to differentiate functionality in different parts of your website. It's very practical and ergonomic to represent non-authenticated users with a non-null object this way. Many web frameworks use this technique.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.