Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AuthComponent static loggedIn #2390

Closed
watermark opened this issue Nov 26, 2013 · 10 comments
Closed

AuthComponent static loggedIn #2390

watermark opened this issue Nov 26, 2013 · 10 comments

Comments

@watermark
Copy link
Contributor

I've run across several instances where having the AuthComponent's "loggedIn" function static would have been useful. It would be a simple change to make the function static.

I think this change is backwards compatible with most code. It may affect people that have called the "loggedIn" function by call_user_func? Perhaps this is a 2.5/3.0 change?

@AD7six
Copy link
Member

AD7six commented Nov 26, 2013

Can you describe one of those instances that isn't served by:

if (Auth::user()) { 

?

@markstory
Copy link
Member

I'd prefer to not add more static methods. They complicate testing, encourage incorrect layering and have generally not worked out well.

@watermark
Copy link
Contributor Author

I have an element that displays different text/links depending on if the user is logged in or not.

I'm currently using the "(bool)AuthComponent::user()" syntax that AD suggested, but it seems like code duplication (however small). It's doing the same thing that "loggedIn" does. If the method shouldn't be static because you can use the Auth::user() function, it could be argued that the "loggedIn" function shouldn't be there at all?

On a side note: please don't take away my static functions mark <3. If we didn't have any static methods in the AuthComponent, this type of element would have to be accomplished cake1.3 style: code in the app controller and sending vars to the element. However proper that 1.3 style may have been, it seems less elegant.

@markstory
Copy link
Member

Sending vars to the template is much better from an application 'structure' point of view though. It also is inline with the separation of concerns ideas that MVC frameworks desire to promote.

While static methods seem elegant, they promote and global variables when the static methods need to refer to state.

@dereuromark
Copy link
Member

@watermark
Dont worry, nobody will be taking away your static methods, at least user() will sure stay for quite a while.

loggedIn() could probably be deprecated in favor of always using user() itself. Otherwise I agree, it would better be static, too, as its merely a wrapper for user().

@dereuromark
Copy link
Member

Closing as the functionality is already fully available with the existing methods?
Or can I move forward with the deprecation of loggedIn() or making it static?

@josegonzalez
Copy link
Member

+1 deprecate loggedIn(). We should ideally have 1 way to do things, and that way should be the best way.

@ADmad
Copy link
Member

ADmad commented Jan 13, 2014

Closing as loggedIn() is now deprecated in 2.5.

@ADmad ADmad closed this as completed Jan 13, 2014
@simkimsia
Copy link

Reading this issue, am I right to say that

$this->Auth->loggedIn() should be replaced by AuthComponent::user() yes?

@ADmad
Copy link
Member

ADmad commented Jul 7, 2014

@simkimsia Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants