PHP: Making an object available to several functions

Question

A beginner question: I don't know how best to structure this bit of code, but basically it goes like this (pseudo code time):

 if (form = submitted) {
     submitted();
 }
 else {
     printForm();
 }

 function submitted() {
     process data from form;
     if(errors = found) {
         print warnings;
         printForm();
     } else {
         submit to database;
     }
 }

 function printForm() {
     print form with databound elements;
 }

I use the following bit of code to create a User object, but it seems weird to call it twice -- once in submitted() and once in printForm(), especially since submitted() calls printForm() if errors are found.

Unfortunately database access is required for processing the data from the form (checking for existing email address, etc), so I have to call the following bit of code in both submitted() and printForm()...

try {
    $db = new Database();
    $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);  
    $user = new User($db);
}
catch (PDOException $e) {
    echo "<p>Error connecting to database: </p>".$e->getMessage();
}

But my instincts tell me that this is bad. Is it? If so, how should I fix it?

FtDRbwLXw6 · Accepted Answer · 2012-02-23 18:22:49Z

3

Use dependency injection:

function submitted(Database $db, User $user) {
    // ...
}

function printForm(Database $db, User $user) {
    // ...
}

try {
    $db = new Database();
    $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);  
    $user = new User($db);
}
catch (PDOException $e) {
    echo "<p>Error connecting to database: </p>".$e->getMessage();
}

submitted($db, $user);
printForm($db, $user);

Of course, it's better to use OOP, as then you wouldn't have to inject the dependencies into every single function:

class Foo {
    protected $db;
    protected $user;

    public function __construct(Database $db, User $user) {
        $this->db = $db;
        $this->user = $user;
    }

    public function submitted() {
        // use $this->db and $this->user here
    }

    public function printForm() {
        // use $this->db and $this->user here
    }
}

try {
    $db = new Database();
    $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);  
    $user = new User($db);
    $foo = new Foo($db, $user);
}
catch (PDOException $e) {
    echo "<p>Error connecting to database: </p>".$e->getMessage();
}

$foo->submitted();
$foo->printForm();

answered Feb 23, 2012 at 18:22

FtDRbwLXw6

29.1k16 gold badges74 silver badges108 bronze badges

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

1 Comment

Chuck Le Butt Over a year ago

I've switched it over to OOP. I should have done it that way from the beginning.

simshaun · Accepted Answer · 2012-02-23 18:22:29Z

There are several ways of making this better...

One would be to initialize $user before the function calls and inject it into each function (dependency injection).

Another way would be to make a singleton of the user instance (although I could see this leading to problems down the road) and retrieve the instance in the function using something like User::instance().

Dependency injection is better in my opinion because it makes your functions easier to test.

Now, on to the rest of the code:

It's typically not good practice to echo anything within a function or class method. You should be returning data instead and handling it outside the function.
Your db instantiation should probably go in a bootstrap file that's included at the top of every script. It's also probably better if your script handles the connection error gracefully (let the user know something went wrong, send yourself an email, stop the rest of the script execution) if the connection fails rather than simply echo'ing an error message.

penartur · Accepted Answer · 2012-02-23 18:24:11Z

0

Although the Singleton pattern is considered to be evil in some cases, it seems that in PHP (with a separate worker process for each request) and in your case it is the easiest way to implement what you want.

So that you have the instance of User who initiated the request available in any script on any page, and you have to initialize the $user object only in one place per project (not in one per script as you were wanting to do).

answered Feb 23, 2012 at 18:24

penartur

9,9325 gold badges41 silver badges51 bronze badges

4 Comments

Gordon Over a year ago

all the problems about coupling, hidden dependencies and global state apply to PHP's Singletons as well. You are right that you will have one instance per request, but that doesnt make Singletons in PHP less evil. It just makes them even more superfluous.

penartur Over a year ago

I agree with you on what is about hidden dependencies; but i don't see the evil behind the global state which is the global state. Sometime it makes a sense to store the context in a singleton (if the context doesn't change over the lifetime of such singleton) instead of carrying the context over all the methods in the callstack.

Gordon Over a year ago

It's a myth that context has to be carryied over all the methods in the callstack just to put dependencies where they are needed. Have a look at the four top videos in youtube.com/results?search_query=The+Clean+Code+Talks&aq=f

penartur Over a year ago

I didn't saw it earlier (will do it now), but the video about the global state and singletons from the first seconds says that primarily the mutable singletons are bad... but in my answer I advised to use immutable singleton.

Collectives™ on Stack Overflow

PHP: Making an object available to several functions

3 Answers 3

1 Comment

Comments

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related