3

I have this User class

class User{
  private    $logged = false;
  private    $id;

  public function User() {
      //> Check if the user is logged in with a cookie-database and set $logged=true;
  }  

  public function isLogged() {}
  public function editPerms() {}

  //> other methods    
}

Well now considering I can't have more than 1 user logged (of course because we are talking for a single http request) in Where should i store the ref of my istance?

This is the case where singleton would be useful but these days everyone say singleton is evil (like static methods).

I could do a $GLOBALS['currentUser'] = new User(); and having it accesible everywhere but I think this is worse than a singleton.

So what Can I do?
Please note I don't need to save this instance between requests. I just need a way to access this instance in my framework within the same request.

If you want to know what i do now for all of my Helper Objects is a Service Container (that's considered as well bad):

function app($class) {      //> Sample
    static $refs = array();

    if (!isset($refs[$class]))
        $refs[$class] = new $class();

    return $refs[$class];
}

//> usage app('User')->methods();

(IE what symfony does)

Nick Weaver
  • 47,228
  • 12
  • 98
  • 108
  • You know, that both the Singleton-Pattern and global variables are only valid within one single request? _Everything_ not stored within the Session, a cookie, a file, or a database will get lost between requests. – KingCrunch May 20 '11 at 23:36
  • yes and I am perfectly fine with it. It's oblivious I need to check for each request if the user is logged in or not. But please don't go OT –  May 20 '11 at 23:37
  • But then your question makes absolutely no sense: You think about where to "save" a user (global or via Singleton), but you cannot save a state this way (because (like I mentioned) it is lost between requests). Also its not possible, that more than one user can be logged in within one single request. – KingCrunch May 20 '11 at 23:38
  • no man I just mean where to save the variable $user (`$user = new User();`) so I can use it everywhere in my framework for that request –  May 20 '11 at 23:39
  • I have changed the title maybe it's more clear the question now –  May 20 '11 at 23:40
  • 7
    It sounds like you are hanging out too much in that chat. These are all cargo cult programming concerns. Declaring any syntax or usage construct as "evil" is baloney. -- Just use a global variable, don't bother with design pattern classification or cramming. If it does reduce complexity and suits the use case, then there's nothing wrong with it. – mario May 20 '11 at 23:44
  • @mario: you have a good point. But that are really some bad pratice we (good developers) should avoid. And I would like to avoid them. –  May 21 '11 at 00:29
  • One benefit of passing the user object around, not using singleton, is if you implement things like permission checking and such in it, it makes it possible for you, as admin, to experience the site as another user just by passing his User object. – runfalk May 25 '11 at 19:00
  • 1
    `that's considered as well bad` - considered by who? SL is good pattern. `globals` is bad. Read Martin Fowler: http://www.martinfowler.com/articles/injection.html#UsingAServiceLocator – OZ_ May 25 '11 at 20:44
  • @OZ_ Always him :( the great Misko here: http://www.youtube.com/watch?v=RlfLCWKxHJ0&feature=player_detailpage#t=536s –  May 25 '11 at 23:15
  • @yes123, well, he is right about one thing - harder to reuse in another code. Yes, it's price of SL. I like DI, really, but I'm pretty sure, in 101%, that it's impossible to write good code with only pure DI in constructors. I know it by practice - you will need SL or setters at least. Not always it will be optimal to create object, which might be not used - it's wasting of memory and performance. Not always it even will be possible, because of chains of dependencies. Try to read Martin Fowler, he said it more smart and his English not as broken as mine :) – OZ_ May 25 '11 at 23:35
  • Lol oz dont worry about your english i am not native too –  May 25 '11 at 23:44

6 Answers6

6

Patterns are supposed to be a helpful guide, like a library of previously successful software abstractions. Too often these days people view patterns as being some kind of religion where things are either "right" or "wrong" regardless of the context of the program.

Think about what you want to achieve and map in out in a way that makes sense to you. Fuggering about with minute distinctions between this pattern and that pattern misses the point, and it won't get your program written. Learn by doing!

HTH.

Robin
  • 4,242
  • 1
  • 20
  • 20
3

Singletons are not evil. Bad usages of singletons are evil. The reason people have come to dislike this pattern so much (even going to the extent of calling it an anti-pattern, whatever that is), is due to improper use:

Too many inexperienced people make a class a singleton when they find they don't need more than one instance of a class. But the question isn't if you need only a single instance of the class, but whether more than one instance would break your code. So ask yourself this question: would your code break if there were more User instances? If not, then maybe you shouldn't bother. :)

There are legitimate uses of singletons. There are those people who fear this pattern like the plague and consider it always to be bad, without realizing that sometimes it can be very helpful. In the words of a much more experinced programmer than me, "singletons are like morphine: they can give you a real boost, but use them the wrong way and they an become a problem themselves". If you want me to go into some details as to when singletons could be a good choice, leave a comment to this answer. :)

Paul Manta
  • 30,618
  • 31
  • 128
  • 208
  • Man you should never say singleton are good. But why not give it a try – dynamic May 25 '11 at 20:31
  • @yes123 Never?! What about logger systems -- the most popular use of singletons? If you refuse to use singletons just because they should _generally_ be avoided, and ignore that sometimes they can actually make your life and everyone else's lives easier, then I don't really know what to say. I'm not the only one in this thread that says singletons aren't always bad. – Paul Manta May 26 '11 at 12:46
  • Logger is usually the case when even all static methods are fine because it's only one-way. But aside from a Logger there aren't many other one-way classk – dynamic May 26 '11 at 12:55
  • 1
    @yes123 "One-way"? I guess you mean state-less classes; or rather classes whose state is irrelevant to anything outside the class. I can think of quite a few other such cases: audio and graphics renderers are two. If done properly, singletons can even save you the trouble of having to (de)initialize the third party libraries you are using for graphics or audio. – Paul Manta May 26 '11 at 13:03
  • @yes123 (Regarding your comment that loggers can simply be made a collection of static function, instead of singletons.) That's true, but is it a better option, or is it simply a sign of the stubbornness of the programmer to use the evil singleton? Singletons (in this case) are much more WYSIWYG: it's clear there's only one instance of the logger. Not to mention that more complex loggers are a bit hard to make with only static functions. – Paul Manta May 26 '11 at 13:09
2

It is always hard to answer architectural questions without the context. In this case it is pretty important how the User objects are persisted (where do they come from?) and how is the client code organized. I will assume a MVC architecture because it's trendy this days. Also I suppose your user objects will have more responsibility as only authentication (you mention some permission control here, but it's still not clear enough).

I would push the authentication responsibility to a service and just pass it around as needed. Here is some sample code.

class AuthenticationService {
    /**
     * @var User
     */
    private $currentUser;

    public function __construct(Request $request) {
        // check if the request has an user identity
        // create a user object or do nothing otherwise
    }

    public function getCurrentUser() {
        return $this->currentUser;
    }
}

class User {
    public function editPerms(){}
}

// the client code
class Controller {
    private $auth;

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

    public function handleRequest() {
        $currentUser = $this->auth->getCurrentUser();

        if ($currentUser === null) { // of course you could use Null Object Pattern
            // no user is logged in
        }

        // do something with the user object
    }
}

So the answer to your question is: you need proper dependency injection through out your whole application. The only object you get from the server is a request. The dependency injection container injects it into the AuthenticationService and the latter gets injected into your controller. No singletons, no static methods, no global variables. The dependencies are tracked in the DI container and are injected as needed. Also the DI container makes sure your service is instantiated only once.

The article "Container-Managed Application Design, Prelude: Where does the Container Belong?" may clarify some DI concepts.

dypsilon
  • 150
  • 6
  • Thanks for your great input. Regarding my context it isnt mvc. It more like a no-framework like rasmus http://toys.lerdorf.com/archives/38-The-no-framework-PHP-MVC-framework.html that's why i dunno where to store these istance – dynamic May 25 '11 at 20:29
  • The code, even in this small article, breaks lots of OOP and application architecture best practices. It is incompatible with two articles you mentioned in the question. Although the main thought of building a lean and reusable pattern just enough to cover your requirements is absolutely valid. – dypsilon May 25 '11 at 20:57
1

Not sure why all the arguing up top. Seems like a perfectly reasonable question to me.

The key here is to use static members of the User class. Static methods are your friends, regardless of what some may say:

class User
{
   private    $logged = false;
   private    $id;

   private static $_currentUser;
   public static function currentUser()
   {
     if (empty(self::$_currentUser))
     {
         @session_start();
         if (array_key_exists('current_user', $_SESSION))
         {
             self::$_currentUser = $_SESSION['current_user'];
         }
         else
         {
           // force login in or whatever else.
           // if you log in, make sure to call User::_setCurrentUser(); 
             return null; //or some special 'empty' user.
         }
     }
     return self::$_currentUser;
  }
  // you may consider making this public, but it is private because it is a bit
  // more secure that way.  
  private static function _setCurrentUser(User $user)
  {
     self::$_currentUser = $user;
     $_SESSION['current_user'] = $user;
  }

  public function User() {
   //> Check if the user is logged in with a cookie-database and set $logged=true;
  }  

  public function isLogged() {}
  public function editPerms() {}

  //> other methods    
}

// Usage
$pUser = User::currentUser();
John Green
  • 13,241
  • 3
  • 29
  • 51
  • Thanks man but i don't need to save it between requests. I just need a a way so I can access globally in my framework just for 1 request – dynamic May 21 '11 at 09:18
  • This does that. Just call User::currentUser(). Since it is static, if it is already set, you're good to go. – John Green May 21 '11 at 09:20
  • Yes But I would avoid to use static methods and singleton. See: http://misko.hevery.com/2008/12/15/static-methods-are-death-to-testability/ for more info – dynamic May 21 '11 at 09:25
  • 1
    This is definitely an opinion, and valid in many things. However, you're asking for data to be shared around the system. You have two other options: Make $user super-global (which is really worse, in many ways), or you make $user an argument into every method you call. But I think Mr. Hevery goes over the top in suggesting that anything static is evil. Even a constructor is inherently static. Every bit of code is inherently procedural at some level. But again, this is one professinal coder's opinion vs another's and in the end, it is all opinion. – John Green May 21 '11 at 09:50
  • Heh. Great argument. I'm no rookie, btw. : ) – John Green May 21 '11 at 10:13
  • Ahh. Just write it in a file and check that one then if nothing else satisfies you. – George Kastrinis May 21 '11 at 10:20
1

The influence of Misko Hevery is pretty strong on me. So is his newable - injectable distinction. A user is not an injectable but a newable. What are the responsibilities of a user: should he be able to tell of himself whether he is logged in or not? There's a post of him where he talks about a similar problem: a credit card and charging it(self?). It happens to be a post about singletons, what you would like to make it:

http://misko.hevery.com/2008/08/17/singletons-are-pathological-liars/

That would leave it to an service to check whether the user is logged in or not, what rights he has on the site.

It also means your architecture would change, your problem will become different (passing around the user?, where is it needed?, how will you have access to the 'checking user is logged in' service, ...).

koen
  • 13,349
  • 10
  • 46
  • 51
  • `passing around the user?` Indeed I am just asking which is the best way to do it – dynamic May 21 '11 at 10:02
  • It depends on your application. Note that u user may have Credentials, Preferences, ... You can use those too. Take special care of the law of demeter. Pass only what is needed to do the job. If you need global access to an object there's something that can be improved. – koen May 21 '11 at 10:12
  • `If you need global access to an object there's something that can be improved` Even Symfony does it: http://symfony.com/doc/2.0/book/service_container.html – dynamic May 21 '11 at 10:16
  • @yes123 It's probably a design trade off for Symfony. See also http://symfony.com/doc/2.0/glossary.html#term-service-container I don't know how it works behind the scenes but it doesn't seem like a real DI container to me. A service should be injected through the constructor to make it clear the class depends on it. It is not clear in the Symfony example. On the other hand They need to cater to all possible uses users of the framework might need. – koen May 21 '11 at 12:09
  • I would prefer the controller to need to implement the __construct and a symfony DI to provide the needed dependencies, eg __construct(Acme\HelloBundle\Mailer $mailer). – koen May 21 '11 at 12:11
  • @koen: take a look of my function app, basically it does it – dynamic May 21 '11 at 19:48
  • @yes123 I don't see much difference between calling app('User') in a method and eg User::getInstance(). Maybe I missed the point of your function. On the other hand I would prefer your version above that of Symfony because you return an object of a certain class, at least you know what is available (the methods of the class). If you bind a class to a name given by the container $container->set('user', 'some_class'), $container->get('user') from reading the client code you don't even know what you will get. – koen May 22 '11 at 00:48
  • Here's more from the creator of symfony with a service container implementation: http://www.slideshare.net/fabpot/dependency-injection-with-php-and-php-53 – koen May 22 '11 at 00:49
0
  1. since everyone else is weighing in on this, singletons are not evil. i even read the "liars" article, and he's using a contrived example of non-modular design and poor dependency inheritance.

i think you should consider a singleton factory pattern, where a singleton factory (Auth) provides a login() method which returns a User class, as well as methods for saving state between HTTP requests on that User.

This will have the benefits of separating the security and session functionality from the User functionality. Additionally using the factory, you can have multiple types of users without the rest of the system needing to understand which object to request before the db is examined

class auth {
    private static $auth = null;
    private $user = null;

    // must use getAuth();
    private __construct(){};

    public getAuth() {
        if (is_null($this->auth) {
             $this->auth = new auth();
        }
        return $this->auth;       
    }

    public function login($user,$pass) {
        ... // check db for user,

        if ($dbrow->user_type == 'admin') {
            $this->user = new admin_user($dbrow);
        } else {
            $this->user = new normal_user($dbrow);
        }

        $this->user->setSession($db->getsession());
    }

    public function getUser() {
        return $this->user;
    }

    public function saveSession() {
        // store $this->user session in db
    }

    public function saveUser() {
        // store $this->user changes in db
    }
    ...
}

the user class itself become a data structure, simply enforcing security and business rules, and maybe formatting some data for output purposes.

class normal_user extends user {
    ... getters and setters
    public function getName() {}
    public function setEmail() {}
    public function setprofile() {}
}

all db, state and security concerns are centralized in the auth. the only way to create a user object (legally) is to run auth->login().

you are still allowed to do

$me = new normal_user();
$me->setName();
echo $me->getName();

but there is no way for a new coder to save this in the db since it's not referenced in $auth->user;

you can then create a function in auth to consume user objects to create new users (on signup)

...
public function create(user $user) {
    // validate $user
    $this->user = $user;
    $this->saveUser();
}
...

you just need to make sure you run the save functions at the end of execution... possibly in a destructor()

simple

David Chan
  • 7,347
  • 1
  • 28
  • 49
  • Basically your class Auth is the container of the unique class User? (Considered you wrote `new normal_user()` this mean class User isn't a singleton) – dynamic May 26 '11 at 09:01
  • oops. sorry user is not the singleton... auth is, edited to reflect that better. User is just a glorified struct. auth is the only way to create/save users and the only method available is login(), to enforce compliance from programmers. – David Chan May 26 '11 at 14:59