Czy dobrą praktyką jest unikanie stałych za pomocą getterów?

Czy dobrą praktyką jest zastępowanie stałych używanych poza klasami przez metody pobierające?

Na przykład, czy lepiej jest użyć if User.getRole().getCode() == Role.CODE_ADMINlub if User.getRole().isCodeAdmin()?

Doprowadziłoby to do tej klasy:

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

Po pierwsze, należy pamiętać, że robienie czegoś takiego entity.underlyingEntity.underlyingEntity.method()jest uważane za zapach kodu zgodnie z prawem Demetera . W ten sposób ujawniasz wiele szczegółów implementacji konsumentowi. I każda potrzeba rozszerzenia lub modyfikacji takiego systemu będzie bardzo boleć.

Biorąc to pod uwagę, zaleciłbym, abyś miał metodę HasRolelub zgodnie z komentarzem CodesInChaos. W ten sposób sposób, w jaki role są wdrażane na użytkowniku, pozostaje szczegółem implementacji dla konsumenta. Bardziej naturalne wydaje się także pytanie użytkownika o jego rolę zamiast pytania o szczegóły jego roli, a następnie podjęcie decyzji w oparciu o to.IsAdminUser

Unikaj także używania strings, chyba że jest to konieczne. namejest dobrym przykładem stringzmiennej, ponieważ zawartość nie jest wcześniej znana. Z drugiej strony, rolejeśli masz dwie wyraźne wartości, które są dobrze znane w czasie kompilacji, lepiej użyj silnego pisania. Właśnie wtedy wchodzi w grę typ wyliczenia ...

Porównać

public bool HasRole(string role)

z

public enum Role { Admin, User }

public bool HasRole(Role role)

Drugi przypadek daje mi o wiele więcej informacji na temat tego, co powinienem przekazywać. Zapobiega mi również błędnym przekazaniem nieważnego stringna wypadek, gdybym nie miał pojęcia o twoich stałych ról.

Następnie jest decyzja, jak będzie wyglądać rola. Możesz użyć enum bezpośrednio zapisanego na użytkowniku:

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

Z drugiej strony, jeśli chcesz, aby twoja rola zachowywała się sama, zdecydowanie powinna ukryć szczegóły dotyczące sposobu decydowania o jej typie:

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

Jest to jednak dość gadatliwe i złożoność wzrasta z każdym dodaniem roli - zwykle tak kończy się kod, gdy próbujesz w pełni przestrzegać prawa Demetera. Powinieneś ulepszyć projekt, w oparciu o konkretne wymagania modelowanego systemu.

Zgodnie z twoim pytaniem, myślę, że lepiej wybrać pierwszą opcję z enum bezpośrednio User. Jeśli potrzebujesz więcej logiki Role, drugą opcję należy uznać za punkt wyjścia.

Wydaje się głupio mieć funkcję sprawdzającą, czy przechowywany kod to kod administratora. Naprawdę chcesz wiedzieć, czy ta osoba jest administratorem. Więc jeśli nie chcesz ujawniać stałych, nie powinieneś również ujawniać, że istnieje kod, a wywołaj metody isAdmin () i isUser ().

To powiedziawszy, „jeśli User.getRole (). GetCode () == Role.CODE_ADMIN” naprawdę jest garstką tylko do sprawdzania, czy użytkownik jest administratorem. Ile rzeczy musi pamiętać deweloper, żeby napisać ten wiersz? Musi pamiętać, że użytkownik ma rolę, rola ma kod, a klasa roli ma stałe dla kodów. To dużo informacji, które dotyczą wyłącznie implementacji.

Oprócz tego, co już napisali inni, należy pamiętać, że użycie stałej ma jeszcze jedną wadę: jeśli coś zmieni sposób, w jaki obchodzisz się z prawami użytkownika, wszystkie te miejsca również muszą zostać zmienione.

A to utrudnia ulepszanie. Może chciałbyś kiedyś mieć superużytkownika , który oczywiście ma również uprawnienia administratora. Dzięki enkapsulacji jest to w zasadzie jedno-liniowy dodatek.

Jest nie tylko krótki i czysty, ale także łatwy w użyciu i zrozumiały. I - może przede wszystkim - trudno się pomylić.

Chociaż w dużej mierze zgadzam się z sugestiami, aby unikać stałych i mieć metodę isFoo()itp., Jeden możliwy kontrprzykład.

Jeśli istnieją setki tych stałych, a wywołania są mało używane, napisanie setek metod isConstant1, isConstant2 może nie być warte wysiłku. W tym szczególnym przypadku użycie stałych jest uzasadnione.

Zauważ, że używanie wyliczeń lub hasRole()pozwala uniknąć potrzeby pisania setek metod, więc jest to najlepszy ze wszystkich możliwych na świecie.

Nie sądzę, aby którakolwiek z przedstawionych opcji była zasadniczo błędna.

Widzę, że nie zasugerowałeś jednej rzeczy, którą nazwałbym całkowicie błędną: twardego kodowania kodów ról w funkcjach spoza klasy Role. To jest:

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

Powiedziałbym, że zdecydowanie się myli. Widziałem programy, które to robią, a następnie otrzymują tajemnicze błędy, ponieważ ktoś źle napisał ciąg. Pamiętam, jak kiedyś programista napisał „zapas”, gdy funkcja sprawdzała „zapas”.

Gdyby było 100 różnych ról, bardzo niechętnie pisałbym 100 funkcji, by sprawdzić każdą możliwą. Prawdopodobnie utworzyłbyś je, pisząc pierwszą funkcję, a następnie kopiując i wklejając ją 99 razy, i ile chcesz postawić na to, że w jednym z tych 99 egzemplarzy zapomnisz zaktualizować test lub dostaniesz się, gdy przejrzałeś listę, więc teraz masz

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

Osobiście unikałbym również łańcuchów połączeń. Wolałbym pisać

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

następnie

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

i w tym momencie dlaczego piszą:

if (user.hasRole(Role.FOOBATER))

Następnie powiadom klasę użytkownika, jak sprawdzić rolę.