Does my class violate the Single Responsibility Principle in SOLID?

Question

I want to ask:

1. Whether the Role class violates Single Responsibility Principle in SOLID ? I think deleteAccount() is not belong to Role class but Role class is way to extend code in the future

2. There are deleteAccount() in Role and Account. That is ok ?

3. Admin and Moderate violates DRY principle ? Because the deleteAccount() was repeated. If yes, how to fix it ?

4. How to implement the deleteAccount() which can delete itself account ? Should I use strategy design pattern to do that ?

    class Account:
       def __init__(self, email, password, security_ans):
           self.status = "Active"
           self.role = Member()
   
   
       def deleteAccount(self, dest_email):
           dest_account = accounts.getByEmail(dest_email) #get account obj from accounts 
           self.role.deleteAccount(self)
   

This is my Role class:

class Role:
def blockAccount(self, account):
    raise NotImplementedError("No Permission")

class Admin(Role):
    def deleteAccount(self, account_obj):
        account_obj.setStatus('Closed')

class Moderator(Role):
    def deleteAccount(self, account_obj):
        account_obj.setStatus('Closed')

class Member(Role):
    def deleteAccount(self, account_obj):
        raise NotImplementedError("No Permission") 

Answer 1

Yes, your Role class is violating the Single Responsibility Principle, because it has knowledge how to delete an account. That knowledge should be reserved to the Account class itself.

The knowledge the belongs in the Role class is which accounts a user with a particular role is allowed to delete. And a single user can have different roles.

For example, Role could handle the permissions like this:

class Role:
  def canDeleteAccount(self, requester, target):
    return False

class Admin(Role):
  def canDeleteAccount(self, requester, target):
    # Admins can delete anyone
    return True 

class Moderator(Role):
  def canDeleteAccount(self, requester, target):
    # Moderators can only delete Members, not Admins or other Moderators
    return target.is_only_member()

class Member(Role):
  def canDeleteAccount(self, requester, target):
    # Members can only delete themselves
    return requester == target

The account deletion logic in Account itself could look something like this

class Account:
  def deleteAccount(self, dest_email, hard_delete=False):
    dest_account = accounts.getByEmail(dest_email) #get account obj from accounts
    can_delete = any(role.canDeleteAccount(self, dest_account) for role in self.roles)
    if can_delete:
      if hard_delete and (Admin in self.roles or dest_account.reputation() < hard_delete_threshold):
        accounts.delete(dest_account)
      else:
        dest_account.setStatus("Closed") 

Note that I extended the deletion logic to allow a hard deletion (removal from the database) for low-reputation accounts or if the deletion is done by a user with the Admin role. The distinction between a hard and soft deletion is not visible for the Role classes.

Also note that Moderator A can delete itself, if that user also has the Member role.

Answer 2

DeleteAccount should not be in any of your examples because a role is not an actor, it is a property. A role has a bunch of rights linked to it. It is just a convenience, it allows you to assign a set of rights to a person. With these rights that person is then allowed or not to initiate a delete operation. The delete operation itself should be a method on an AccountManager class that can create, modify and delete accounts.

So your main problem is that your are thinking of roles as persons. They are not, you still need a person class, which could be Member because member is not a role anyway. Just keep member, do not make it descend from role and instead add a list of roles as a property to member.