Funkcje jednowierszowe, które są wywoływane tylko raz

Rozważ funkcję bez parametrów ( edytuj: niekoniecznie), która wykonuje pojedynczy wiersz kodu i jest wywoływana tylko raz w programie (choć nie jest niemożliwe, że będzie ona potrzebna ponownie w przyszłości).

Może wykonać zapytanie, sprawdzić niektóre wartości, zrobić coś z wyrażeniem regularnym ... wszystko niejasne lub „hacky”.

Uzasadnieniem tego byłoby uniknięcie trudnych do odczytania ocen:

if (getCondition()) {
    // do stuff
}

gdzie getCondition()jest funkcja jednowierszowa.

Moje pytanie brzmi: czy to dobra praktyka? Wydaje mi się to w porządku, ale nie wiem o długim okresie ...

Zależy od tej jednej linii. Jeśli wiersz jest czytelny i zwięzły sam w sobie, funkcja może nie być potrzebna. Prosty przykład:

void printNewLine() {
  System.out.println();
}

OTOH, jeśli funkcja nadaje dobre imię linii kodu zawierającej np. Złożone, trudne do odczytania wyrażenie, jest to dla mnie całkowicie uzasadnione. Przemyślany przykład (podzielony na wiele wierszy dla czytelności tutaj):

boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isFemale() 
        && (taxPayer.getNumberOfChildren() > 2 
        || (taxPayer.getAge() > 50 && taxPayer.getEmployer().isNonProfit()));
}

Tak, można to wykorzystać w celu spełnienia najlepszych praktyk. Na przykład lepiej, aby funkcja o wyraźnej nazwie działała, nawet jeśli ma tylko jeden wiersz, niż mieć ten wiersz kodu w większej funkcji i potrzebować komentarza wyjaśniającego, co robi. Również sąsiednie wiersze kodu powinny wykonywać zadania na tym samym poziomie abstrakcji. Kontrprzykład byłby coś w rodzaju

startIgnition();
petrolFlag |= 0x006A;
engageChoke();

W takim przypadku zdecydowanie lepiej jest przesunąć środkową linię do rozsądnie nazwanej funkcji.

Myślę, że w wielu przypadkach taka funkcja jest dobrym stylem, ale możesz rozważyć lokalną zmienną logiczną jako alternatywę w przypadkach, gdy nie musisz używać tego warunku gdzieś w innych miejscach, np .:

bool someConditionSatisfied = [complex expression];

To da podpowiedź dla czytnika kodów i pozwoli zaoszczędzić na wprowadzeniu nowej funkcji.

Oprócz odpowiedzi Piotra , jeśli ten warunek może wymagać aktualizacji w pewnym momencie w przyszłości, poprzez włączenie go do sposobu, w jaki sugerujesz, że będziesz miał tylko jeden punkt edycji.

Idąc za przykładem Piotra, jeśli to

boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isFemale() 
        && (taxPayer.getNumberOfChildren() > 2 
        || (taxPayer.getAge() > 50 && taxPayer.getEmployer().isNonProfit()));
}

staje się tym

boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isMutant() 
        && (taxPayer.getNumberOfThumbs() > 2 
        || (taxPayer.getAge() > 123 && taxPayer.getEmployer().isXMan()));
}

Wykonujesz jedną edycję, która jest aktualizowana uniwersalnie. Jeśli chodzi o łatwość konserwacji, jest to plus.

Pod względem wydajności najbardziej optymalizujące kompilatory usuwają wywołanie funkcji i wstawiają mały blok kodu. Optymalizacja czegoś takiego może faktycznie zmniejszyć rozmiar bloku (poprzez usunięcie instrukcji potrzebnych do wywołania funkcji, powrotu itp.), Więc zwykle jest to bezpieczne nawet w warunkach, które w przeciwnym razie mogłyby uniemożliwić wstawianie.

Oprócz czytelności (lub jej uzupełnienia) pozwala to pisać funkcje na odpowiednim poziomie abstrakcji.

To zależy. Czasami lepiej jest zawrzeć wyrażenie w funkcji / metodzie, nawet jeśli jest to tylko jedna linia. Jeśli czytanie jest skomplikowane lub potrzebujesz go w wielu miejscach, uważam to za dobrą praktykę. W dłuższej perspektywie łatwiej jest utrzymać, ponieważ wprowadziłeś jeden punkt zmiany i lepszą czytelność .

Czasami jednak jest to po prostu coś, czego nie potrzebujesz. Jeśli mimo wszystko wyrażenie jest łatwe do odczytania i / lub pojawia się w jednym miejscu, nie zawijaj go.

Myślę, że jeśli masz ich tylko kilka, to jest w porządku, ale problem pojawia się, gdy jest ich dużo w kodzie. A kiedy kompilator działa lub interpitor (w zależności od używanego języka) Przechodzi do tej funkcji w pamięci. Powiedzmy, że masz 3 z nich. Nie sądzę, aby komputer to zauważył, ale jeśli zaczniesz mieć 100 takich drobiazgów, system będzie musiał zarejestrować funkcje w pamięci, które są wywoływane tylko raz, a następnie nie są niszczone.

Niedawno zrobiłem dokładnie to samo w aplikacji, którą refaktoryzowałem, aby jasno określić rzeczywiste znaczenie kodu bez komentarzy:

protected void PaymentButton_Click(object sender, EventArgs e)
    Func<bool> HaveError = () => lblCreditCardError.Text == string.Empty && lblDisclaimer.Text == string.Empty;

    CheckInputs();

    if(HaveError())
        return;

    ...
}

Przeniesienie tego wiersza do dobrze nazwanej metody ułatwia odczytanie kodu. Wiele innych już o tym wspomniało („kod samodokumentujący”). Inną zaletą przeniesienia go do metody jest to, że ułatwia testowanie jednostkowe. Kiedy jest izolowany we własnej metodzie i testowany jednostkowo, możesz być pewien, że jeśli / kiedy zostanie znaleziony błąd, nie będzie w tej metodzie.

Istnieje już wiele dobrych odpowiedzi, ale warto wspomnieć o specjalnym przypadku .

Jeśli instrukcja jednowierszowa wymaga komentarza i jesteś w stanie jednoznacznie zidentyfikować (co oznacza: nazwa) jej cel, rozważ wyodrębnienie funkcji przy jednoczesnym rozszerzeniu komentarza do dokumentu API. W ten sposób sprawisz, że wywołanie funkcji będzie szybsze i łatwiejsze do zrozumienia.

Co ciekawe, to samo można zrobić, jeśli obecnie nie ma nic do zrobienia, ale potrzebny jest komentarz przypominający o rozszerzeniach (w najbliższej przyszłości ¹⁾ ), więc to,

def sophisticatedHello():
    # todo set up
    say("hello")
    # todo tear down

równie dobrze mógł się w to zmienić

def sophisticatedHello():
    setUp()
    say("hello")
    tearDown()

¹⁾ powinieneś być tego pewien (patrz zasada YAGNI )

Jeśli język to obsługuje, zwykle używam do tego celu anonimowych funkcji z etykietami.

someCondition = lambda p: True if [complicated expression involving p] else False
#I explicitly write the function with a ternary to make it clear this is a a predicate
if (someCondition(p)):
    #do stuff...

IMHO jest to dobry kompromis, ponieważ daje korzyść z czytelności, ponieważ nie ma skomplikowanego wyrażenia zaśmiecającego ifwarunek, jednocześnie unikając zaśmiecania przestrzeni nazw globalnych / pakietów małymi etykietami wyrzucania. Dodatkową zaletą jest to, że funkcja „definicja” jest odpowiednia tam, gdzie jest używana, co ułatwia modyfikację i odczyt definicji.

Nie muszą to być tylko funkcje predykatów. Lubię też umieszczać powtarzające się płytki kotła w małych funkcjach takich jak ta (działa szczególnie dobrze w przypadku generowania listy pythonowej bez zaśmiecania składni nawiasu). Na przykład poniższy uproszczony przykład podczas pracy z PIL w Pythonie

#goal - I have a list of PIL Image objects and I want them all as grayscale (uint8) numpy arrays
im_2_arr = lambda im: array(im.convert('L')) 
arr_list = [im_2_arr(image) for image in image_list]