Code-Poets / coding-style

0 stars 0 forks source link

return, return None #6

Open Jakub89 opened 6 years ago

Jakub89 commented 6 years ago

Powinniśmy jasno określić w naszym coding-style, jak i kiedy korzystać z różnych zakończeń funkcji:

A: Funkcja, która nic nie zwraca i ma tylko jedną ścieżkę wykonania

1.

     def foo():
        print('foo')

2.

    def foo():
       print('foo')
       return

3.

    def foo():
       print('foo')
       return None

B: Funkcja, która nic nie zwraca i ma wiele ścieżek wykonania

4.

     def baz(value):
        if value is not None:
            print('baz')

5.

     def baz(value):
        if value is None:
            return

        print('baz')

6.

     def baz(value):
        if value is None:
            return None

        print('baz')

C: Funkcja, która zwraca wynik

7.

     def bar(value):
        if value is None:
            return None

        return value * 3 - 2

8.

     def bar(value):
        if value is not None:
            return value * 3 - 2

9.

     def bar(value):
        if value is not None:
            return value * 3 - 2

        return

10.

     def bar(value):
        if value is not None:
            return value * 3 - 2

        return None
pawelkisielewicz commented 6 years ago

Jak najbardziej jestem za. Mam jeszcze kolejną sugestię w tym temacie. Często korzystamy w sygnaturach funkcji i metod z określenia typów pól, które mają być do niej przekazywane, np.

def foo(key : bytes, deadline: int):

natomiast rzadko piszemy, jaki typ jest zwracany. Określajmy może tam gdzie się da również typ wartości zwracanej, np.

def foo(key : bytes, deadline: int)->str:

cameel commented 6 years ago

@pawelkisielewicz To powinno zostać zgłoszone jako osobny issue.

cameel commented 6 years ago

@Jakub89 Miałeś źle wcięcia zrobione w opisie przez co bloki z kodem spadały poniżej. Poprawiłem. Zobacz w źródłach jak to powinno było być zrobione.

cameel commented 6 years ago

Moim zdaniem kwestia jest bardziej skomplikowana. 1 - 3 to tylko szczególny przypadek i najbardziej trywialna sytuacja - procedura, która nic nie zwraca i ma jedną ścieżkę wykonania. Dodałem więc do opisu trochę więcej przypadków.

Moje preferencje są takie:

Uzasadnienie:

rwrzesien commented 6 years ago

Zasadniczo:

robią to samo, za każdym razem przypisanie wywołania takiej funkcji do zmiennej da None.

Moim zdaniem: A - 1 B - 4 C - 8

cameel commented 6 years ago

Moim zdaniem: (...) C - 8

Ja jestem zdecydowanie przeciwny opcji 8. Pamiętam na poczatku projektu, że mieliśmy sporo dziur w logice przez konstrukcje typu:

if A:
    ...
    if B:
        ...
        return response X
else
    ...
    return response Y

Funkcja była pełna ifów i miała z założenia nie zwracać None. Znacznie trudniej jest to dostrzec jeżeli to zwrócenie None jest niejawne. W tej wersji nie jesteś w stanie tego przegapić:

if A:
    ...
    if B:
        ...
        return response X
    else:
        return None
else
    ...
    return response Y
cameel commented 6 years ago

Swoją drogą, to jest nawet wprost opisane w PEP8:

Be consistent in return statements. Either all return statements in a function should return an expression, or none of them should. If any return statement returns an expression, any return statements where no value is returned should explicitly state this as return None, and an explicit return statement should be present at the end of the function (if reachable).

Yes:

def foo(x):
    if x >= 0:
        return math.sqrt(x)
    else:
        return None

def bar(x):
    if x < 0:
        return None
    return math.sqrt(x)

No:

def foo(x):
    if x >= 0:
        return math.sqrt(x)

def bar(x):
    if x < 0:
        return
    return math.sqrt(x)
dybi commented 6 years ago

Zgadzam się z @cameel: A - 1, B - 4 albo 5 (w zależności od sytuacji: w prostych funkcjach opcja 4, w dłuższych - 5 dla wcześniejszego wyjścia z funkcji) C - 7 albo 10 (wg klucza jak wyżej)

Funkcje można podzielić z grubsza na dwie główne kategorie (nomenklatura moja, por: https://stackoverflow.com/questions/721090/what-is-the-difference-between-a-function-and-a-procedure): 1) Procedury - funkcje, których zadaniem jest coś zrobić (nie zwracają wyniku), np. logowanie czegoś, printowanie, zapisanie na dysku/do bazy/do pliku, itp. 2) Funkcje właściwe - funkcje, które zwracają wynik, np. pobranie czegoś, funkcje matematyczne, itp.

IMHO, procedury nie powinny używać return, chyba że jest z nich kilka wyjść - wtedy powinno się użyć gołego return Funkcje właściwe powinny w każdym punkcie wyjścia używać return warość, nawet jeśli ta wartość to None.

cameel commented 6 years ago

OK. Skoro to jest już określone w PEP8 to czy chcemy dodawać to do naszego coding style? A może powinniśmy po prostu mieć sekcję która daje dodatkowe wyjaśnienia dla rzeczy, które w PEP8 już są?

dybi commented 6 years ago

@cameel, dobre pytanie. Wydaje się, że ludzie albo nie czytają PEP8, albo nie rozumieją, albo nie pamiętają ;) Jeśli to trzecie, to niewile pomoże nasza pisanina :P

Moim zdaniem, skoro problem jest realny (co jakiś czas wraca w PR-ach), to albo trzeba zacząć robić kartkówki z PEP8, albo te najbardziej palące kwestie mimo wszysko opisywać na wiki.