Sprawdzanie, czy metoda zwraca false: przypisać wynik do zmiennej tymczasowej, czy bezpośrednio wywołać metodę w warunkowym?

9

Czy dobrą praktyką jest wywoływanie metody, która zwraca wartości prawda lub fałsz w instrukcji if?

Coś takiego:

private void VerifyAccount()
{
    if (!ValidateCredentials(txtUser.Text, txtPassword.Text))
    {
        MessageBox.Show("Invalid user name or password");
    }
}

private bool ValidateCredentials(string userName, string password)
{
    string existingPassword = GetUserPassword(userName);
    if (existingPassword == null)
        return false;

    var hasher = new Hasher { SaltSize = 16 };
    bool passwordsMatch = hasher.CompareStringToHash(password, existingPassword);

    return passwordsMatch;
}

czy lepiej jest przechowywać je w zmiennej, a następnie porównywać je, używając innych wartości jak to

bool validate = ValidateCredentials(txtUser.Text, txtPassword.Text);
if(validate == false){
    //Do something
}

Mam nie tylko na myśli .NET, mam na myśli pytanie we wszystkich językach programowania, tak się składa, że ​​użyłem .NET jako przykładu

KyelJmD
źródło
3
Jeśli używasz zmiennej tymczasowej, pisz if (!validate)zamiast if (validate == false).
Philip
12
Nazwałbym tę funkcję czymś w rodzaju „CredentialsAreValid ()”, więc wiesz, że powinna zwracać wartość bool, ale poza tym tak, to dobra praktyka
Zachary K
3
IsValidCredentials, chociaż gramatycznie niewygodny, jest powszechnym formatem do wskazywania logicznej wartości zwracanej.
zzzzBov,
@Philip jaka jest różnica między if (! Validate) a tym if ((validate) ??)
KyelJmD
!jest operatorem „NIE”, neguje każde wyrażenie logiczne. Podobnie if (!validate)jest z odwrotnością if (validate). Instrukcja if zostanie wprowadzona, jeśli validatenie jest prawdziwa.
Philip

Odpowiedzi:

26

Tak jak w przypadku wszystkich tych rzeczy, to zależy.

Jeśli nie zamierzasz użyć wyniku połączenia, ValidateCredentialsnie ma potrzeby (oprócz celów debugowania) przechowywania wyniku w zmiennej lokalnej. Jeśli jednak sprawi, że kod będzie bardziej czytelny (a przez to łatwiejszy w utrzymaniu), będzie mieć do niego zmienną.

Kod nie będzie wymiernie mniej wydajny.

ChrisF
źródło
1
czy ten typ jest czytelny? jak ten, który zrobiłem powyżej
KyelJmD
@KyelJmD: To zależy od kultury, w której programujesz. W miejscach, w których programowałem, !ValidateCredentialsjest bardziej czytelny, ponieważ wyraźnie mówi, co robi w kodzie. To bardzo jasne. Jeśli funkcja, którą wywołujesz, nie ma nazwy „samodokumentującej”, być może lepiej będzie użyć zmiennej. Na obecnym etapie zaleciłbym pominięcie zmiennej i pozostanie przy posiadanym kodzie samodokumentującym.
Joel Etherton,
ValidateCredentials nie jest przede wszystkim czytelny - w jaki sposób nazwa mówi ci, co oznacza wynik? O wiele lepiej CredentialsAreValid lub CredentialsAreInvalid.
gnasher729,
9

Dlaczego warto korzystać z dodatkowej zmiennej? Wolę zastosować pierwsze podejście, jest bardziej czytelne i proste.

Diego Pacheco
źródło
4

Czy dobrą praktyką jest wywoływanie metody, która zwraca wartości prawda lub fałsz w instrukcji if?

Tak, jeśli warunek nie jest wystarczająco prosty, aby można go było wstawić i zapewnić, aby był czytelny.

czy lepiej jest przechowywać je w zmiennej, a następnie porównywać je, używając innych wartości jak to

Powinieneś to zrobić tylko wtedy, gdy używasz wartości w wielu miejscach lub potrzebujesz jej, aby kod był bardziej czytelny. W przeciwnym razie przypisanie do zmiennej nie jest konieczne. Niepotrzebny kod jest w najlepszym wypadku marnotrawstwem, aw najgorszym przypadku źródłem wady.

dietbuddha
źródło
2

Pokazać...

Ponieważ chodzi przede wszystkim o KISS , nie ma potrzeby tworzenia dodatkowej zmiennej, gdy można się bez niej obejść. Nie trzeba też pisać więcej ... kiedy nie ma takiej potrzeby.

Ale ponieważ wysychasz , jeśli później dzwonisz ValidateCredentialsi piszesz ValidateCredentials(txtUser.Text, txtPassword.Text), wiesz, że powinieneś był utworzyć dodatkową zmienną.

kowsheek
źródło
2

Tak, zwykle dobrze jest stosować takie metody, jak gdyby warunki. Jest to pomocne, jeśli nazwa metody wskazuje, że metoda zwraca bool; na przykład CanValidateCredentials. W językach w stylu C metoda ta często przyjmuje postać przedrostków Is i Can, aw języku Ruby z „?” przyrostek.

Christian Horsdal
źródło
1
Dobra uwaga na temat nazw metod, ale nigdy nie zaczynam nazwy metody od „if”. Następnie kod brzmi „jeśli jeśli”. "Can" jest ok: if (cat.canOpenCanOfCatFood()).
kevin cline
Dzięki, @Kevin. Miałem na myśli „nie”, „jeśli”. Zredagowałem odpowiedź
Christian Horsdal
1

Jest jeszcze jeden problem, który nie został jeszcze wskazany: ocena zwarcia . Jaka jest różnica między tymi dwoma fragmentami kodu?

Ex # 1:

if(foo() && bar() && baz())
{
    quz();
}

Ex # 2:

bool isFoo = foo();
bool isBar = bar();
bool isBaz = baz();
if(isFoo && isBar && isBaz)
{
    quz();
}

Te dwa fragmenty kodu wydają się robić to samo, ale jeśli twój język obsługuje ocenę zwarć, te dwa fragmenty są różne. Ocena zwarcia oznacza, że ​​kod oceni minimum potrzebne do spełnienia lub niepowodzenia warunku. Jeśli przykład # 1, jeśli foo () zwraca false, bar () i baz () nie są nawet obliczane. Jest to przydatne, jeśli baz () jest długo działającym wywołaniem funkcji, ponieważ można je pominąć, jeśli foo () zwraca false lub foo () zwraca true, a bar () zwraca false.

Tak nie jest w przykładzie 2. foo (), bar () i baz () są zawsze oceniane. Jeśli oczekujesz, że bar () i baz () wykażą skutki uboczne, będziesz mieć problemy z przykładem 1. Przykład 1 powyżej jest faktycznie równoważny z tym:

Ex # 3:

if(foo())
{
    if(bar())
    {
        if(baz())
        {
            quz();
        }
    }
}

Pamiętaj o tych różnicach w swoim kodzie, wybierając między przykładami 1 i 2.

user2023861
źródło
1

Rozwijając nieco kwestię czytelności ...

Mogę wymyślić dwa dobre powody, aby przechowywać wynik w zmiennej:

  1. Jeśli zamierzasz używać warunku więcej niż jeden raz, zapisanie go w zmiennej oznacza, że ​​musisz wywołać tę funkcję tylko raz. (Zakłada się, że przechowywana wartość jest nadal aktualna; jeśli trzeba ją ponownie przetestować, oczywiście nie ma to zastosowania).

  2. Przechowywanie go w zmiennej poprawia czytelność, nadając warunkowi nazwę bardziej znaczącą niż to, co widzisz w wywołaniu funkcji.

Na przykład:

bool foo_is_ok = is_ok(foo);
if (foo_is_ok) ...

nie poprawia czytelności, ale to:

bool done_processing = feof(file1) && feof(file2);
if (done_processing) ...

prawdopodobnie tak, ponieważ nie jest to od razu oczywiste, że feof(file1) && feof(file2)oznacza to, że zakończyliśmy przetwarzanie.

passwordsMatchZmienna w tej kwestii jest prawdopodobnie lepszym przykładem niż moje.

Zmienne jednorazowe są użyteczne, jeśli nadają sensowną nazwę pewnej wartości. (Jest to oczywiście korzystne dla ludzkiego czytelnika).

Keith Thompson
źródło
Myślę, że wolę zostawić komentarz niż wprowadzić done_processingzmienną. W końcu nazwa done_processingpełni funkcję komentarza. A prawdziwy komentarz pozwala mi powiedzieć jedno lub dwa słowa o tym, dlaczego ten warunek sygnalizuje, że zakończyliśmy przetwarzanie. Nie dlatego, że jestem świetnym komentatorem, prawdopodobnie nie zrobiłbym żadnego z prawdziwego kodu ...
cmaster - przywróć Monikę
@cmaster: Jednym z problemów z komentarzami jest to, że mogą bardzo łatwo zsynchronizować się z kodem. Zmienna o nazwie done_processingto bardziej trwały sposób wyrażania intencji.
Keith Thompson,
+1 za punkt 2 - czasami dobrze nazwany temp var naprawdę wyjaśnia rzeczy.
user949300 30.10.16