Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Better name for a method than "isThis()"

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.

like image 734
Dan Avatar asked Nov 29 '22 12:11

Dan


1 Answers

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.

like image 69
Ben Smith Avatar answered Dec 05 '22 14:12

Ben Smith