Make User::verifyAuthTypeCookie static

[mlocati]

It's called statically and doesn't contain any reference to $this, so it's safe to mark it as static

[KorvinSzanto]

It'd be better to move this and other auth validators into its own new
construct.

On Tue, Apr 14, 2015, 11:05 PM Michele Locati notifications@github.com
wrote:

It's called statically and doesn't contain any reference to $this, so

it's safe to mark it as static

You can view, comment on, or merge this pull request online at:

#2250
Commit Summary

• Make User::verifyAuthTypeCookie static
• PSR

File Changes

• M web/concrete/src/User/User.php
  https://github.com/concrete5/concrete5-5.7.0/pull/2250/files#diff-0
  (78)

Patch Links:

• https://github.com/concrete5/concrete5-5.7.0/pull/2250.patch
• https://github.com/concrete5/concrete5-5.7.0/pull/2250.diff

—
Reply to this email directly or view it on GitHub
#2250.

[mlocati]

It'd be better to move this and other auth validators into its own new
construct.

I absolutely don't understand what you mean 😖

[KorvinSzanto]

I feel like the fact that this doesn't rely on an instance of \User is more an indicator that this method shouldn't be on the User class than that this method should be static. Converting this method to static prevents things like mocking in tests and makes it harder to override, so maybe a \Concrete\Core\Authentication\Validator that you can get an instance of from \Core::make('authentication/validator').

That makes it easy to override, makes it mockable (since this is a very likely candidate for mocking), and categorizes the functionality to better align with SRP.

[mlocati]

I thought that this kind of stuff was demanded to the core team 😉. What I'm doing with these pull requests is to lower the number of warnings thrown by PHP (see #1309): I modified the Whoops error handler to store all the warnings to a sqlite DB and then I'm fixing the most recurring ones.

[KorvinSzanto]

Ah fair, just my half asleep morning 2c