¿Es una buena práctica evitar constantes usando getters?

¿Es una buena práctica reemplazar las constantes utilizadas fuera de las clases por los captadores?

Como ejemplo, ¿es mejor usar if User.getRole().getCode() == Role.CODE_ADMINo if User.getRole().isCodeAdmin()?

Eso llevaría a esta clase:

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
    }
}

En primer lugar, tenga en cuenta que hacer algo como entity.underlyingEntity.underlyingEntity.method()se considera un olor a código según la Ley de Demeter . De esta manera, está exponiendo muchos detalles de implementación al consumidor. Y cada necesidad de extensión o modificación de dicho sistema dolerá mucho.

Entonces, dado eso, te recomiendo que tengas un método HasRoleo IsAdminmétodo según el Usercomentario de CodesInChaos. De esta forma, la forma en que se implementan los roles en el usuario sigue siendo un detalle de implementación para el consumidor. Y también se siente más natural preguntarle al usuario cuál es su rol en lugar de preguntarle sobre los detalles de su rol y luego decidir basándose en eso.

También evite usar strings a menos que sea necesario. namees un buen ejemplo de stringvariable porque los contenidos son desconocidos de antemano. Por otro lado, algo así como roledonde tiene dos valores distintos que son bien conocidos en el momento de la compilación, será mejor que use una escritura fuerte. Ahí es donde entra en juego el tipo de enumeración ...

Comparar

public bool HasRole(string role)

con

public enum Role { Admin, User }

public bool HasRole(Role role)

El segundo caso me da mucha más idea de lo que debería estar pasando. También me impide pasar erróneamente un inválido stringen caso de que no tuviera idea de las constantes de su rol.

Lo siguiente es la decisión sobre cómo se verá el papel. Puede usar enum directamente almacenado en el usuario:

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 otro lado, si desea que su rol tenga un comportamiento en sí mismo, definitivamente debería ocultar nuevamente los detalles de cómo se decide su tipo:

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();
    }
}

Sin embargo, esto es bastante detallado y la complejidad aumentaría con cada adición de roles; por lo general, así es como termina el código cuando intenta adherirse completamente a la Ley de Demeter. Debe mejorar el diseño, en función de los requisitos concretos del sistema que se está modelando.

Según su pregunta, supongo que será mejor que elija la primera opción con enum directamente User. Si necesita más lógica sobre el Role, la segunda opción debe considerarse como un punto de partida.

Parece una tontería tener una función para verificar si el código que se almacena es el código de administrador. Lo que realmente quieres saber es si esa persona es un administrador. Entonces, si no desea exponer las constantes, tampoco debe exponer que hay un código y llamar a los métodos isAdmin () y isUser ().

Dicho esto, "if User.getRole (). GetCode () == Role.CODE_ADMIN" es realmente un puñado solo para verificar que un usuario es un administrador. ¿Cuántas cosas debe recordar un desarrollador para escribir esa línea? Debe recordar que un usuario tiene un rol, un rol tiene un código y la clase Role tiene constantes para los códigos. Esa es una gran cantidad de información que se trata únicamente de la implementación.

Además de lo que otros ya han publicado, debe tener en cuenta que usar la constante directamente tiene otro inconveniente: si algo cambia la forma en que maneja los derechos de usuario, todos esos lugares también deben cambiarse.

Y hace que sea horrible mejorarlo. Tal vez le gustaría tener un tipo de superusuario en un punto, que obviamente también tiene derechos de administrador. Con la encapsulación, es básicamente una línea para agregar.

No solo es corto y limpio, también es fácil de usar y comprender. Y, tal vez sobre todo, es difícil equivocarse.

Si bien estoy en gran medida de acuerdo con las sugerencias para evitar constantes y tener un método isFoo(), etc., un posible contraejemplo.

Si hay cientos de estas constantes, y las llamadas son poco utilizadas, puede que no valga la pena escribir cientos de métodos isConstant1, isConstant2. En este caso particular e inusual, el uso de las constantes es razonable.

Tenga en cuenta que el uso de enumeraciones o hasRole()evita la necesidad de escribir cientos de métodos, por lo que es el mejor de todos los mundos posibles.

No creo que ninguna de las opciones que presentó sea fundamentalmente incorrecta.

Veo que no ha sugerido lo único que llamaría totalmente erróneo: codificar los códigos de función en funciones fuera de la clase Role. Es decir:

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

Yo diría que definitivamente está mal. He visto programas que hacen esto y luego obtienen errores misteriosos porque alguien escribió mal la cadena. Recuerdo una vez que un programador escribió "stock" cuando la función estaba buscando "Stock".

Si hubiera 100 roles diferentes, sería muy reacio a escribir 100 funciones para verificar cada posible. Presumiblemente los crearía escribiendo la primera función y luego copiándola y pegándola 99 veces, y cuánto desea apostar a que en una de esas 99 copias olvidaría actualizar la prueba o se saldría por una cuando revisaste la lista, así que ahora tienes

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

Personalmente, también evitaría las cadenas de llamadas. Prefiero escribir

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

luego

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

y en ese punto por qué escribir nota:

if (user.hasRole(Role.FOOBATER))

Luego, deje que la clase Usuario sepa cómo verificar un rol.