É uma boa prática evitar constantes usando getters?

É uma boa prática substituir constantes usadas fora das classes por getters?

Como exemplo, é melhor usar if User.getRole().getCode() == Role.CODE_ADMINou if User.getRole().isCodeAdmin()?

Isso levaria a essa classe:

class Role {
    constant CODE_ADMIN = "admin"
    constant CODE_USER = "user"

    private code

    getRoleCode() {
       return Role.code
    }

    isCodeAdmin () {
       return Role.code == Role.CODE_ADMIN
    }

    isCodeUser () {
       return Role.code == Role.CODE_USER
    }
}

Primeiro, observe que fazer algo como entity.underlyingEntity.underlyingEntity.method()é considerado um cheiro de código de acordo com a Lei de Demeter . Dessa forma, você está expondo muitos detalhes de implementação ao consumidor. E cada necessidade de extensão ou modificação de um sistema desse tipo prejudicará muito.

Portanto, eu recomendo que você tenha um método HasRoleou IsAdminno Usercomentário de CodesInChaos. Dessa maneira, a maneira como as funções são implementadas no usuário permanece sendo o detalhe da implementação para o consumidor. E também parece mais natural perguntar ao usuário qual é o seu papel, em vez de perguntar sobre detalhes de seu papel e decidir com base nisso.

Evite também usar strings, a menos que seja necessário. nameé um bom exemplo de stringvariável porque o conteúdo é desconhecido de antemão. Por outro lado, algo como roleonde você tem dois valores distintos que são bem conhecidos no momento da compilação, é melhor usar uma digitação forte. É aí que o tipo de enumeração entra em jogo ...

Comparar

public bool HasRole(string role)

com

public enum Role { Admin, User }

public bool HasRole(Role role)

O segundo caso me dá muito mais idéia do que eu deveria estar passando. Também me impede de passar erroneamente um inválido string, caso eu não tenha idéia das constantes de sua função.

Em seguida, está a decisão sobre como será o papel. Você pode usar enum diretamente armazenado no usuário:

public enum Role
{
    Admin,
    User
}

public class User
{
    private Role _role;

    public bool HasRole(Role role)
    {
        return _role == role;
    }

    // or
    public bool IsAdmin()
    {
        return _role == Role.Admin;
    }
}

Por outro lado, se você deseja que seu papel tenha um comportamento próprio, ele definitivamente deve ocultar novamente os detalhes de como seu tipo está sendo decidido:

public enum RoleType
{
    User,
    Admin
}

public class Role
{
    private RoleType _roleType;

    public bool IsAdmin()
    {
        return _roleType == RoleType.Admin;
    }

    public bool IsUser()
    {
        return _roleType == RoleType.User;
    }

    // more role-specific logic...
}

public class User
{
    private Role _role;

    public bool IsAdmin()
    {
        return _role.IsAdmin();
    }

    public bool IsUser()
    {
        return _role.IsUser();
    }
}

No entanto, isso é bastante detalhado e a complexidade aumentaria a cada adição de função - geralmente é assim que o código termina quando você tenta aderir totalmente à Lei de Demeter. Você deve melhorar o design, com base nos requisitos concretos do sistema que está sendo modelado.

De acordo com sua pergunta, acho melhor você ir com a primeira opção com enum diretamente User. Se você precisar de mais lógica Role, a segunda opção deve ser considerada como ponto de partida.

Parece idiota ter uma função para verificar se o código armazenado é o código de administrador. O que você realmente quer saber é se essa pessoa é um administrador. Portanto, se você não deseja expor as constantes, também não deve expor que existe um código e chame os métodos isAdmin () e isUser ().

Dito isto, "se User.getRole (). GetCode () == Role.CODE_ADMIN" é realmente um punhado apenas para verificar se um usuário é um administrador. Quantas coisas um desenvolvedor precisa lembrar para escrever essa linha? Ele ou ela deve se lembrar de que um usuário tem uma função, uma função tem um código e a classe Role tem constantes para códigos. Muitas informações são puramente sobre a implementação.

Além do que outras pessoas já postaram, lembre-se de que o uso direto da constante tem outra desvantagem: se algo mudar a maneira como você lida com os direitos do usuário, todos esses locais também precisam ser alterados.

E torna horrível melhorar. Talvez você queira ter um tipo de superusuário em algum momento, que obviamente também tenha direitos de administrador. Com o encapsulamento, é basicamente uma linha a ser adicionada.

Não é apenas curto e limpo, também é fácil de usar e entender. E - talvez acima de tudo - é difícil errar.

Embora eu concorde amplamente com as sugestões para evitar constantes e ter um método isFoo()etc., um possível contra-exemplo.

Se houver centenas dessas constantes e as chamadas forem pouco usadas, pode não valer a pena o esforço de escrever centenas de métodos isConstant1, isConstant2. Nesse caso particular e incomum, o uso das constantes é razoável.

Observe que o uso de enums ou hasRole()evita a necessidade de escrever centenas de métodos, por isso é o melhor de todo o mundo possível.

Não acho que nenhuma das opções que você apresentou esteja fundamentalmente errada.

Vejo que você não sugeriu a única coisa que eu chamaria totalmente errada: codificar os códigos de função em funções fora da classe Role. Isso é:

if (user.getRole().equals("Administrator")) ...

Eu diria que está definitivamente errado. Eu já vi programas que fazem isso e, em seguida, recebem erros misteriosos porque alguém digitou errado a string. Lembro-me de uma vez que um programador escreveu "estoque" quando a função estava verificando "estoque".

Se houvesse 100 funções diferentes, eu ficaria muito relutante em escrever 100 funções para verificar todas as possíveis. Presumivelmente, você os criaria escrevendo a primeira função, copiando e colando 99 vezes, e quanto deseja apostar que em uma dessas 99 cópias você esquecerá de atualizar o teste ou sairá uma quando você percorreu a lista, agora você tem

public bool isActuary() { return code==Role.ACTUARY; }
public bool isAccountant() { return code==Role.ACTUARY; }
... etc ...

Pessoalmente, eu também evitaria as ligações. Prefiro escrever

if (user.getRole().equals(Role.FOOBATER))

então

if (user.getRole().getRoleCode()==Role.FOOBATER_CODE)

e nesse ponto, por que a nota escreve:

if (user.hasRole(Role.FOOBATER))

Em seguida, informe à classe Usuário como verificar uma função.