0

I have 2 class in my CMS and there are crud functions in "User Class". How can I use database class in another class. I know they both work. But I was previously transferring with the "extends" method. But every time I created an object, the database connection was duplicated. So "extends" was fault for me. I don't want to fault again.

So which is best solution? Do any of these solutions duplicate the database connection?

databaseclass.php:

class database
{
    public $connect;

    function __construct() {
        $this->connect();
    }
    public function connect() {
        $this->connect = new PDO('mysql:host=' . SERVER . ';dbname=' . DB_NAME . ';charset=utf8', DB_USER, DB_PASS);
    }
}

userclass.php:

class user
{

    function getUser() {
        # select an user codes
    }
}
  1. Solution - Injection database class

    class user
    {
    
        function getUser($database) {
            $mysql_command = $database->connection->prepare('SELECT * FROM users WHERE id = 1');
            $mysql_command->execute();
            return $mysql_command->fetch(PDO::FETCH_ASSOC);
        }
    }
    
  2. Solution - Global database class (I want that but everybody suggest first solution) (I want because this solution does not require me to inject each time I create a user object) userclass.php:

    class user
    {
    
        function getUser() {
            global $database;
            $mysql_command = $database->connection->prepare('SELECT * FROM users WHERE id = 1');
            $mysql_command->execute();
            return $mysql_command->fetch(PDO::FETCH_ASSOC);
        }
    }
    
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
halfvita
  • 13
  • 5
  • 1
    Have a look around for dependency injection, this is a common solution to your problem. – Nigel Ren Feb 28 '20 at 19:15
  • Neither 1 or 2 but 1 is closer. Why inject a database into each method when you can inject it into the whole class through a constructor parameter? – Your Common Sense Feb 29 '20 at 09:39
  • @YourCommonSense Sorry, what I thought was exactly the same as you said. But typing fast made me make mistakes. But what I meant was to inject into the constructor parameter. – halfvita Feb 29 '20 at 18:57

1 Answers1

0

I added a third option here and gave some personal opinion about the others:

Solution a) Pass as parameter to your methods

This way you always need to pass it inside your functions which makes the API of your class less clean. Imagine we want to get a user by id. This could look like: getUserById($database, 123). This will require everywhere to know how to fetch the databse. I would try to avoid this.

Solution b) Use it as global

I would try avoid global wherever I can. My first argument would be that it makes your class less testable and can change the behaviour dependent on the context. Here is a nice post about it with more examples: https://stackoverflow.com/a/5166527/12880865

Solution c) Use dependency injection with the __constructor.

I would add another way of solving it by using dependency injection at constructor level already. This way you can use getUsers() or getUserById($id) without passing the database all the time. This way your class defines it's own dependencies very clearly for the user of this class.

this solution does not require me to inject each time I create a user object

To avoid this you could use container which could resolves your class dependencies automatically when you try to retrieve them.

Example with your code:

class user
{
    protected $database;

    public function __construct(database $database) {
        $this->database = $database;
    }

    function all() {
        # select an user codes
        $this->database->query('....');
    }

    function getById($id) {
        # select an user codes
        $this->database->query('....', ['id' => $id]);
    }
}

which you can then inject like this:

$database = new Database();

// Here you'll pass your database connection into the repository
$users = new UserRepository($database); 
$users->all(); // returns an array of all users
$users->getById(123); // returns a specific user by id

// Here you'll pass the same database connection into another repository
$posts = new PostRepository($database);
$posts->all(); // returns an array of all posts
$posts->getById(123); // returns a specific post by id
Christoph Kluge
  • 1,947
  • 8
  • 23
  • First of all thank you for your knowledge, suggestion and sharing. I have two questions. First: why you use interface? (database $database) Is there something it makes easy? I ask because I don't know completely. If I work alone, do interfaces matter? Second question: Will this line of code not duplicate the database connection? "$users = new Users(new Database());" – halfvita Feb 28 '20 at 21:06
  • For some reason you failed with the only proper method. new Users(new Database()); is absolutely NOT how do you provide a database connection to the User class. @halfvita surely it will – Your Common Sense Feb 29 '20 at 09:34
  • @YourCommonSense can you elaborate a bit more instead of telling people that they failed? My assumption is that the author's `Users` class is his idea of having a repository-pattern. – Christoph Kluge Feb 29 '20 at 09:42
  • 1
    @halfvita (1) It's not an interface but it would make totally sense to use an interface `DatabaseInterface`. I'm using the type-hint to force this parameter to be of a specific type. (2) You're right. I've adopted my example code to show how you can re-use the database object in two separate objects. – Christoph Kluge Feb 29 '20 at 09:53
  • @YourCommonSense man thank you really! you found logical answers to both old and new questions. And thank you again Christoph Kluge. So guys, the most logical method is injection and "$database = new Database(); $users = new UserRepository($database);" is the healthiest use. Am I Right? – halfvita Feb 29 '20 at 16:19
  • Yes, exactly that – Your Common Sense Feb 29 '20 at 16:52
  • @YourCommonSense Would you consider adding a discussion of DI and repositories to your website? I see from these comments that I’m missing some detail somewhere, because I am missing what is wrong with `new User(new Database)`. I recognize that this would create a new connection for each instantiation, but if a singleton *connection* was injected into each `Database`, I fail to see the downside (beyond the controversial singleton). – Tim Morton Feb 29 '20 at 17:27
  • @TimMorton singleton is an antipattern and besides, you nver instantiate a singleton, it has a private constructor, so with new database it cannot be a singleton. – Your Common Sense Feb 29 '20 at 17:42