For this specific example of the "can reset a password", I'd recommend using composition over inheritance (in this case, inheritance of an interface/contract). Because, by doing this:
class Foo : IResetsPassword {
//...
}
You're immediately specifying (at compile time) that your class 'can reset a password'. But, if in your scenario, the capability's presence is conditional and depends on other things, then you can't specify things at compile time anymore. Then, I suggest doing this:
class Foo {
PasswordResetter passwordResetter;
}
Now, at runtime, you can check if myFoo.passwordResetter != null
before doing this operation. If you want to decouple stuff even more (and you plan to add many more capabilities), you could:
class Foo {
//... foo stuff
}
class PasswordResetOperation {
bool Execute(Foo foo) { ... }
}
class SendMailOperation {
bool Execute(Foo foo) { ... }
}
//...and you follow this pattern for each new capability...
UPDATE
After I read some other answers and comments from OP I understood the question is not about the compositional solution. So I think the question is about how to better identify capabilities of objects in general, in a scenario like below:
class BaseAccount {
//...
}
class GuestAccount : BaseAccount {
//...
}
class UserAccount : BaseAccount, IMyPasswordReset, IEditPosts {
//...
}
class AdminAccount : BaseAccount, IPasswordReset, IEditPosts, ISendMail {
//...
}
//Capabilities
interface IMyPasswordReset {
bool ResetPassword();
}
interface IPasswordReset {
bool ResetPassword(UserAccount userAcc);
}
interface IEditPosts {
bool EditPost(long postId, ...);
}
interface ISendMail {
bool SendMail(string from, string to, ...);
}
Now, I'll try to analyze all options mentioned:
OP second example:
if (account.CanResetPassword)
((IResetsPassword)account).ResetPassword();
else
Print("Not allowed to reset password with this account type!");
Let's say this code is receiving some base account class (eg: BaseAccount
in my example); this is bad since it's inserting booleans in the base class, polluting it with code that makes no sense at all to be there.
OP first example:
if (account is IResetsPassword)
((IResetsPassword)account).ResetPassword();
else
Print("Not allowed to reset password with this account type!");
To answer the question, this is more appropriate than the previous option, but depending on the implementation it will break L principle of solid, and probably checks like this would be spread through the code and make further maintenance more difficult.
CandiedOrange's anser:
account.ResetPassword(authority);
If this ResetPassword
method is inserted in BaseAccount
class, then it's also polluting the base class with inappropriate code, like in OP's second example
Snowman's answer:
AccountManager.resetPassword(otherAccount, adminAccount.getAccessToken());
This is a good solution, but it considers that the capabilities are dynamic (and might change over time). However, after I read several comments from OP I guess the talk here is regarding polymorphism and statically defined classes (although the example of accounts intuitivelly points to the dynamic scenario). EG: in this AccountManager
example the check for permission would be queries to DB; in the OP question the checks are attempts of casting the objects.
Another suggestion from me:
Use Template Method pattern for high level branching. The class hierarchy mentioned is kept as it is; we only create more appropriate handlers for the objects, in order to avoid casts and inappropriate properties/methods poluting the base class.
//Template method
class BaseAccountOperation {
BaseAccount account;
void Execute() {
//... some processing
TryResetPassword();
//... some processing
TrySendMail();
//... some processing
}
void TryResetPassword() {
Print("Not allowed to reset password with this account type!");
}
void TrySendMail() {
Print("Not allowed to reset password with this account type!");
}
}
class UserAccountOperation : BaseAccountOperation {
UserAccount userAccount;
void TryResetPassword() {
account.ResetPassword(...);
}
}
class AdminAccountOperation : BaseAccountOperation {
AdminAccount adminAccount;
override void TryResetPassword() {
account.ResetPassword(...);
}
void TrySendMail() {
account.SendMail(...);
}
}
You could bind the operation to the appropriate account class by using a dictionary/hashtable, or perform run-time operations using extension methods, use dynamic
keyword, or as last option use only one cast in order to pass the account object to the operation (in this case the number of casts is only one, at the beginning of the operation).
CanResetPassword
will only be true when something implementsIResetsPassword
. Now you are making a property determine something the type system should control (and ultimately will when you preform the cast).