Powinieneś zawsze przekazywać niezbędne minimum do funkcji w takich przypadkach

Załóżmy, że mam funkcję IsAdminsprawdzającą, czy użytkownik jest administratorem. Powiedzmy również, że sprawdzanie przez administratora odbywa się poprzez dopasowanie identyfikatora użytkownika, nazwy i hasła do jakiejś reguły (nieistotnej).

W mojej głowie są do tego dwie możliwe sygnatury funkcji:

public bool IsAdmin(User user);
public bool IsAdmin(int id, string name, string password);

Najczęściej wybieram drugi rodzaj podpisu, myśląc, że:

• Podpis funkcji daje czytelnikowi znacznie więcej informacji
• Logika zawarta w funkcji nie musi wiedzieć o Userklasie
• Zwykle powoduje to nieco mniej kodu wewnątrz funkcji

Czasami jednak kwestionuję to podejście i zdaję sobie sprawę, że w pewnym momencie stałoby się ono niewygodne. Gdyby na przykład funkcja odwzorowała między dziesięć różnych pól obiektu na wynikowy bool, oczywiście wysłałbym cały obiekt. Ale oprócz tego surowego przykładu nie widzę powodu, aby przekazać rzeczywisty obiekt.

Byłbym wdzięczny za wszelkie argumenty za każdym stylem, a także za wszelkie ogólne spostrzeżenia, które możesz zaoferować.

Programuję zarówno w stylach zorientowanych obiektowo, jak i funkcjonalnie, więc pytanie powinno dotyczyć wszystkich idiomów.

Ja osobiście wolę pierwszą metodę just IsAdmin(User user)

Jest o wiele łatwiejszy w użyciu, a jeśli twoje kryteria dla IsAdmin zmieniają się później (być może w oparciu o role lub isActive), nie musisz wszędzie przepisywać podpisu metody.

Jest to również prawdopodobnie bezpieczniejsze, ponieważ nie reklamujesz, jakie właściwości określają, czy użytkownik jest administratorem, czy nie, ani nie przekazujesz wszędzie właściwości hasła. A syrion ma rację, że co się stanie, gdy twój idnie pasuje do name/ password?

Długość kodu wewnątrz funkcji nie powinna mieć tak naprawdę znaczenia, pod warunkiem, że metoda spełnia swoje zadanie, i wolałbym mieć krótszy i prostszy kod aplikacji niż kod metody pomocniczej.

Pierwsza sygnatura jest lepsza, ponieważ umożliwia hermetyzację logiki związanej z użytkownikiem w samym Userobiekcie. Nie jest korzystne logiczne konstruowanie krotki „id, name, password” rozrzuconej o bazie kodu. Co więcej, komplikuje to isAdminfunkcję: co się stanie, jeśli ktoś przejdzie coś id, co nie pasuje name, na przykład? Co zrobić, jeśli chcesz sprawdzić, czy dany Użytkownik jest administratorem w kontekście, w którym nie powinieneś znać jego hasła?

Ponadto, jako uwaga na twój trzeci punkt na korzyść stylu z dodatkowymi argumentami; może powodować „mniej kodu w funkcji”, ale dokąd idzie ta logika? Nie może tak po prostu zniknąć! Zamiast tego rozprzestrzenia się po każdym miejscu, w którym wywoływana jest funkcja. Aby zapisać pięć wierszy kodu w jednym miejscu, zapłaciłeś pięć wierszy za użycie funkcji!

Pierwszy, ale nie (tylko) z powodów, które podali inni.

public bool IsAdmin(User user);

To jest bezpieczne. Zwłaszcza, że ​​użytkownik jest typem, który sam zdefiniowałeś, nie ma prawie żadnej możliwości transponowania lub zmiany argumentów. Wyraźnie działa na użytkownikach i nie jest jakąś ogólną funkcją akceptującą int i dwa ciągi. Jest to duża część sensu używania języka bezpiecznego dla typu, takiego jak Java lub C #, jak wygląda twój kod. Jeśli pytasz o konkretny język, możesz dodać tag do tego języka do swojego pytania.

public bool IsAdmin(int id, string name, string password);

Tutaj będą działać dowolne int i dwa ciągi. Co uniemożliwia Ci transpozycję nazwy i hasła? Drugi to więcej pracy w użyciu i więcej okazji do błędów.

Przypuszczalnie obie funkcje wymagają wywołania user.getId (), user.getName () i user.getPassword () albo wewnątrz funkcji (w pierwszym przypadku), albo przed wywołaniem funkcji (w drugim), więc ilość sprzężenia jest taka sama w obu przypadkach. W rzeczywistości, ponieważ ta funkcja jest poprawna tylko dla użytkowników, w Javie wyeliminowałbym wszystkie argumenty i uczyniłbym ją metodą instancji dla obiektów użytkownika:

user.isAdmin();

Ponieważ ta funkcja jest już ściśle powiązana z użytkownikami, warto uczynić ją częścią tego, czym jest użytkownik.

PS Jestem pewien, że to tylko przykład, ale wygląda na to, że przechowujesz hasło. Zamiast tego powinieneś przechowywać tylko bezpieczny hash kryptograficznie bezpieczny. Podczas logowania dostarczone hasło powinno być zakodowane w ten sam sposób i porównane z przechowywanym hashem. Należy unikać wysyłania haseł w postaci zwykłego tekstu. Jeśli naruszysz tę regułę i isAdmin (int Str Str) miałby zalogować nazwę użytkownika, to jeśli transponujesz nazwę i hasło w swoim kodzie, hasło może zostać zarejestrowane zamiast tego, co stwarza problem bezpieczeństwa (nie zapisuj haseł w logach ) i jest kolejnym argumentem przemawiającym za przekazaniem obiektu zamiast jego części składowych (lub za pomocą metody klasy).

Jeśli twój wybrany język nie przekazuje struktur wartości, (User user)wariant wydaje się lepszy. Co jeśli pewnego dnia zdecydujesz się zeskrobać nazwę użytkownika i po prostu użyć identyfikatora / hasła, aby zidentyfikować użytkownika?

Takie podejście nieuchronnie prowadzi do długich i przesadzonych wywołań metod lub niespójności. Czy przekazanie 3 argumentów jest w porządku? A może 5? Czy 6? Po co o tym myśleć, jeśli przekazywanie obiektu jest tanie pod względem zasobów (prawdopodobnie nawet tańsze niż przekazywanie 3 argumentów)?

I (w pewnym sensie) nie zgadzam się, że drugie podejście daje czytelnikowi więcej informacji - intuicyjnie wywołanie metody pyta „czy użytkownik ma uprawnienia administratora”, a nie „czy dana kombinacja id / nazwa / hasło ma uprawnienia administratora”.

Tak więc, chyba że przekazanie Userobiektu spowoduje skopiowanie dużej struktury, pierwsze podejście jest dla mnie czystsze i bardziej logiczne.

Prawdopodobnie miałbym oba. Pierwszy jako zewnętrzny interfejs API, z pewną nadzieją na stabilność w czasie. Drugi jako prywatna implementacja wywoływana przez zewnętrzny interfejs API.

Gdybym później musiał zmienić regułę sprawdzania, po prostu napisałbym nową funkcję prywatną z nową sygnaturą wywoływaną przez zewnętrzną.

Ma to tę zaletę, że łatwość zmiany wewnętrznej implementacji. Jest też bardzo często, że w pewnym momencie możesz potrzebować obu funkcji jednocześnie i wywoływać jedną lub drugą w zależności od zmiany kontekstu zewnętrznego (na przykład możesz mieć starych koegzystujących i nowych użytkowników z różnymi polami).

Jeśli chodzi o tytuł pytania, w obu przypadkach podajesz minimum niezbędnych informacji. Użytkownik wydaje się być najmniejszą strukturą danych związanych z aplikacją zawierającą informacje. Identyfikator trzech pól, hasło i nazwa wydają się być najmniejszymi faktycznie potrzebnymi danymi implementacyjnymi, ale tak naprawdę nie są to obiekty na poziomie aplikacji.

Innymi słowy Jeśli miałbyś do czynienia z bazą danych, użytkownik byłby rekordem, podczas gdy identyfikator, hasło i login byłyby polami w tym rekordzie.

Istnieje jeden punkt ujemny, aby wysłać klasy (typy referencyjne) do metod, a mianowicie usterka związana z przypisywaniem masy . Wyobraź sobie, że zamiast IsAdmin(User user)metody mam UpdateUser(User user)metodę.

Jeśli Userklasa ma wywołaną właściwość Boolean IsAdmin, a jeśli nie sprawdzę jej podczas wykonywania mojej metody, jestem podatny na masowe przypisywanie. GitHub został zaatakowany przez ten sam podpis w 2012 roku.

Wybrałbym takie podejście:

public bool IsAdmin(this IUser user) { /* ... */ }

public class User: IUser { /* ... */ }

public interface IUser
{
    string Username {get;}
    string Password {get;}
    IID Id {get;}
}

• Użycie typu interfejsu jako parametru dla tej funkcji umożliwia przekazywanie obiektów użytkownika, definiowanie fałszywych obiektów użytkownika do testowania i daje większą elastyczność zarówno w zakresie używania, jak i utrzymania tej funkcji.

• Dodałem także słowo kluczowe, thisaby funkcja była metodą rozszerzenia wszystkich klas IUser; w ten sposób możesz pisać kod w bardziej przyjazny dla użytkownika sposób i używać tej funkcji w zapytaniach Linq. Jeszcze prostsze byłoby zdefiniowanie tej funkcji w IUser i User, ale zakładam, że istnieje jakiś powód, dla którego zdecydowałeś się umieścić tę metodę poza tą klasą?

• Używam niestandardowego interfejsu IIDzamiast, intponieważ pozwala to na nowo zdefiniować właściwości twojego identyfikatora; na przykład, jeśli kiedyś trzeba zmienić od intdo long, Guidlub coś innego. To pewnie przesada, ale zawsze dobrze jest uelastycznić rzeczy, aby nie ograniczać się do wczesnych decyzji.

• Uwaga: Twój obiekt IUser może znajdować się w innym obszarze nazw i / lub w zestawie niż klasa User, więc masz możliwość całkowitego oddzielenia kodu użytkownika od kodu IsAdmin, przy czym współużytkują one tylko jedną bibliotekę, do której istnieje odwołanie. Dokładnie to, co robisz, wymaga sprawdzenia, w jaki sposób podejrzewasz użycie tych przedmiotów.

Zastrzeżenia:

Aby odpowiedzieć, skupię się na kwestii stylu i zapomnę o tym, że identyfikator, login i zwykłe hasło to dobry zestaw parametrów, których należy używać z punktu widzenia bezpieczeństwa. Powinieneś być w stanie ekstrapolować moją odpowiedź na każdy podstawowy zestaw danych, którego używasz, aby dowiedzieć się o uprawnieniach użytkownika ...

Uważam również za user.isAdmin()równoważne z IsAdmin(User user). To jest wybór dla ciebie.

Odpowiadać:

Moje zalecenie dotyczy tylko jednego użytkownika:

public bool IsAdmin(User user);

Lub użyj obu, zgodnie z:

public bool IsAdmin(User user); // calls the private method below
private bool IsAdmin(int id, string name, string password);

Powody publicznej metody:

Pisząc metody publiczne, zwykle dobrze jest pomyśleć o ich zastosowaniu.

W tym konkretnym przypadku powodem, dla którego zwykle wywołujesz tę metodę, jest ustalenie, czy dany użytkownik jest administratorem. To metoda, której potrzebujesz. Naprawdę nie chcesz, aby każdy dzwoniący musiał wybrać odpowiednie parametry do przesłania (i ryzykować błędy z powodu duplikacji kodu).

Powody zastosowania metody prywatnej:

Metoda prywatna otrzymująca tylko minimalny zestaw wymaganych parametrów może być czasem dobra. Czasem nie tak bardzo.

Jedną z dobrych rzeczy w dzieleniu tych metod jest to, że możesz wywołać wersję prywatną z kilku metod publicznych w tej samej klasie. Na przykład, jeśli chcesz (z jakiegokolwiek powodu) mieć nieco inną logikę Localuseri RemoteUser(może istnieje ustawienie włączania / wyłączania zdalnych administratorów?):

public bool IsAdmin(Localuser user); // Just call private method
public bool IsAdmin(RemoteUser user); // Check if setting is on, then call private method
private bool IsAdmin(int id, string name, string password);

Dodatkowo, jeśli z jakiegoś powodu musisz upublicznić metodę prywatną ... możesz. To tak proste, jak zmiana privatena public. Może nie wydawać się to wielką zaletą w tym przypadku, ale czasami tak naprawdę jest.

Ponadto podczas testów jednostkowych będziesz w stanie wykonać znacznie więcej testów atomowych. Zwykle bardzo miło jest nie tworzyć pełnego obiektu użytkownika, aby uruchomić testy tej metody. A jeśli chcesz mieć pełny zasięg, możesz przetestować oba połączenia.

public bool IsAdmin(int id, string name, string password);

jest zły z wymienionych powodów. To nie znaczy

public bool IsAdmin(User user);

jest automatycznie dobry. W szczególności, jeśli użytkownik jest obiekt mający metod takich jak: getOrders(), getFriends(), getAdresses(), ...

Możesz rozważyć refaktoryzację domeny za pomocą typu UserCredentials zawierającego tylko identyfikator, nazwę, hasło i przekazanie go do IsAdmin zamiast wszystkich niepotrzebnych danych użytkownika.