4

I have one class called OAuthLogin that supports the login of a user via OAuth. The website supports also a "traditional" login process, without OAuth. The two flows share a lot of code, where I need to differentiate them sometimes.

I have a static method OAuthLogin::isThis() that returns a boolean whether the current login flow is OAuth or not (by checking session variables and URL parameters).

I don't like the name of the method but I can't think of a better one - I guess that is a common concept, therefore there should be some kind of pattern.

I don't like OAuthLogin::isThisOAuthLogin() because is redundant.

I would like to avoid Login::isThisOAuth because I would like to keep all the code in the OAuthLogin class.

Should I go for OAuthLogin::is()? Anything better than that?

Thanks.

4

5 回答 5

22

Your OAuthLogin class should only have one responsibility, and that is to enable a user to login using OAuth; this class should have no knowledge of the "traditional" login process. A person who sees this class name (e.g. StackOverflow users!) will assume that this class is only responsible for login functionality using OAuth.

As your two login processes share a lot of code, then you really should refactor your code so that you have a base class with the common code, and then have separate classes for OAuth and Traditional login which will both inherit from the base class. When your code executes you should then instantiate the login class that is appropriate for that user's session.

Also as your OAuthLogin class is static then how will it be able to handle many users logging-in at the same time? Hence another good reason to refactor it so that it is not static.

If you absolutely cannot refactor, then without seeing your code, it sounds as if the OAuthLogin class is really a mediator i.e. it encapsulates how a set of classes interact (in this case your login classes). So instead of using the name "OAuthLogin" you could call it "LoginMediator". You could then have the properties:

LoginMediator.isOauthLogin

and

LoginMediator.isTraditionalLogin

to distinguish between the different types of login which the mediator is using for that particular session. Though instead of using the word "Traditional" replace this with the other authentication mechanism you actually use (e.g. HTTP Basic Authentication, HTTP Digest Authentication, HTTPS Client Authentication etc.)

Note how I have chosen intention-revealing names for these properties. If a stranger was to read your code (e.g. me!) they would struggle to understand the purpose of "is()" and "isThis()" from just the method signature.

However, in the long run I really do recommend that you refactor your code so that you have classes with discrete responsibilities, as you will find that naming methods will be far easier as a result.

于 2013-11-08T00:06:25.177 回答
0

I would add a method to the base class which just returns the type of the login.

Class (pseudo-code)

class Login
  method class
    return self.class # Returns the current class.
  end
end

Usage would be (also pseudo-code):

if currentLogin.class == OAuthLogin
  # ..
else
  # ..
end

This would let you add more types later on, without having to add type-specific methods for each login type, leaving the control flow outside of your classes.

于 2013-11-07T12:01:52.277 回答
0

I suggest one of:

OAuthLogin::isCurrent()
OAuthLogin::isCurrentLogin()
OAuthLogin::isCurrentFlow()
OAuthLogin::isCurrentLoginFlow()
OAuthLogin::isActive()
OAuthLogin::isActiveLogin()
OAuthLogin::isActiveFlow()
OAuthLogin::isActiveLoginFlow()
于 2013-11-09T01:53:21.837 回答
0

How about OAuthLogin::isUsed()?

于 2013-11-14T08:08:39.803 回答
-1

I suggest OAUthLogin::isLoggedIn().

于 2013-11-08T01:38:19.363 回答