Bardzo krótkie metody

0

Ciekawi mnie co doświadczony programista sądzi o takich metodach?
Dobry kod powinien być czytelny i zrozumiały po samych nazwach klas/metod/zmiennych, ale czy powinno się praktykować 1-liniowe metody?

Dla przykładu:

public static boolean fileExist(String fileName) {
        return new File(fileName).isFile();
    }

    public static boolean isEven(int number) {
        return number % 2 == 0;
    }

    public static float averageLineLength(int charsSum, int linesSum) {
        return charsSum / linesSum;
    }
2

Ja uważam że nie ma co przeginać, bo:

  1. Metody powinny być krótkie, ale w tym przypadku możesz zataić co masz na myśli, każdy myśli że isEven zwróci liczbę parzystą ale jak coś skopiesz w tej metodę to wprowadzisz wiele osób w błąd.
  2. Nie powiem żeby to było dużo czytelniejsze (w przypadku dwóch ostatnich metod)
  3. Metody statyczne to w cale nie jakieś mistrzostwo w programowaniu obiektowym.
  4. Wydajność.... niby nic wielkiego - jedna linijka, ale jednak trzeba odłożyć skok do metody i obsługę wyników na stosie/stercie.

Dobre są 4-5 linijkowe metody, te nico dłuższe też, ale jednolinijkowece?

3

pytanie czy naprawdę potrzeba tutaj osobnej metody, czy nie lepiej po prostu napisać number % 2 == 0 itp. niż komplikować i używać metody.

Myślę, że jak zwykle odpowiedzią jest "to zależy".

w przypadku isEven zapewne zysk będzie zerowy, ale już np. zenkapsulowanie sprawdzania czy plik istnieje w postaci metody fileExist może mieć sens, jeśli będziesz chciał pobawić się potem w polimorfizm i np. obsługę różnych systemów plików. Ale to sam musisz sie za każdym razem zastanowić co ma większy sens w danym przypadku.

W przypadku averageLineLength to szczerze powiedziawszy coś tu jest nie halo. Taka funkcja chyba powinna przyjmować jakiegoś rodzaju kolekcję stringów a nie dwa inty? Mnie się wydaje, że zły poziom abstrakcji tu przyjąłeś.

17

Zależy od długości i stopnia wykorzystania. Np 1-linijkowe metody używane w jednym miejscu zwykle nie miałyby sensu - takie metody można zastąpić po prostu zmiennymi. Jednak w wielu przypadkach jednolinijkowe metody mają sens, bo opisują co robi dany kod.

Taki kod nie ma sensu, bo nazwa metody nie wnosi nic co jest natychmiastowo widoczne w ciele metody:

void multiplyNumbers(int a, int b) {
  return a * b;
}

Ale taka metoda już coś wnosi:

boolean userCanEditItem(User user, Item item) {
  return user.isAdmin() || user.manageableItemTypes.contains(item.type);
}

bo zdecydowanie szybciej jestem w stanie zrozumieć to:

if (!userCanEditItem(user, item)) {
  throw new Error("illegal operation");
}

niż to:

if (!(user.isAdmin() || user.manageableItemTypes.contains(item.type))) {
  throw new Error("illegal operation");
}

Nie muszę się domyślać, że chodzi o prawa dostępu, bo wynika to wprost z nazwy metody.

1
Slepiec napisał(a):

Metody powinny być krótkie, ale w tym przypadku możesz zataić co masz na myśli, każdy myśli że isEven zwróci liczbę parzystą ale jak coś skopiesz w tej metodę to wprowadzisz wiele osób w błąd.

Jak skopiesz w innym miejscu, to też wprowadzisz w błąd.

Slepiec napisał(a):

Metody statyczne to w cale nie jakieś mistrzostwo w programowaniu obiektowym.

Do sprawdzenia podzielności liczby nie trzeba kontenera DI i fabryki strategii.

Slepiec napisał(a):

Wydajność.... niby nic wielkiego - jedna linijka, ale jednak trzeba odłożyć skok do metody i obsługę wyników na stosie/stercie.

Tym się zajmuje kompilator, poza tym szanujące się platformy i tak nie zrobią żadnego wywołania metody.

0
Afish napisał(a):
Slepiec napisał(a):

Metody powinny być krótkie, ale w tym przypadku możesz zataić co masz na myśli, każdy myśli że isEven zwróci liczbę parzystą ale jak coś skopiesz w tej metodę to wprowadzisz wiele osób w błąd.

Jak skopiesz w innym miejscu, to też wprowadzisz w błąd.

Tak ale jak skopiesz w metodzie która powinna być oczywista a nie jest, to chyba gorzej.

Slepiec napisał(a):

Metody statyczne to w cale nie jakieś mistrzostwo w programowaniu obiektowym.

Do sprawdzenia podzielności liczby nie trzeba kontenera DI i fabryki strategii.

Do oczywistych rzeczy nie trzeba takich metod wcale.

Slepiec napisał(a):

Wydajność.... niby nic wielkiego - jedna linijka, ale jednak trzeba odłożyć skok do metody i obsługę wyników na stosie/stercie.

Tym się zajmuje kompilator, poza tym szanujące się platformy i tak nie zrobią żadnego wywołania metody.

Tak, owszem, obyśmy mieli do czynienia tylko z szanującymi się platformami :D

1
Slepiec napisał(a):

Tak ale jak skopiesz w metodzie która powinna być oczywista a nie jest, to chyba gorzej.

Jak dla mnie w obu miejscach jest to samo, a jak nie wyjdzie na testach i przeglądzie kodu, to nieużycie małej metody i tak nie pomoże.

Slepiec napisał(a):

Do oczywistych rzeczy nie trzeba takich metod wcale.

Nie chodzi o oczywistość, tylko o czytelność. Dla jednego użycie operatora modulo będzie czytelne, inny użyje koniunkcji bitowej, a jeszcze inny metody z biblioteki standardowej. Zależy od zespołu.

Slepiec napisał(a):

Tak, owszem, obyśmy mieli do czynienia tylko z szanującymi się platformami :D

To wtedy masz o wiele większe problemy, niż jakieś tam wywołanie metody, do tego platforma może wreszcie zacząć się szanować i wprowadzać optymalizacje, a ręczne inlineowanie metod wcale nie jest optymalizacją.

6

Metody powinny być krótkie, ale w tym przypadku możesz zataić co masz na myśli, każdy myśli że isEven zwróci liczbę parzystą ale jak coś skopiesz w tej metodę to wprowadzisz wiele osób w błąd.

Nazwa isEven jest zła, bo jest zbyt oczywista? To może nazwa hasMagicalProperty byłaby lepsza? Wtedy nikt nie byłby przesadnie pewny siebie, ba każdy byłby podejrzliwy i sprawdzałby ciało metody.

Jeśli metoda jest skopana to znaczy, że:

  • była słabo wytestowana (prawdopodobnie wcale, bo łatwe metody łatwo się testuje)
  • wystarczy poprawić kod w jednym miejscu, a nie w wielu

Aczkolwiek robienie metody isEven to już nieco przegięcie. Myślę, że przez takie kiepskie przykłady dyskusja nad krótkimi metodami idzie w złym kierunku.

0

Po prostu zmieść całą metodę w jednym ekranie lub mniej. Tworzenie miliona jednolinijkowych metod to przesada w drugą stronę, bo nie bardzo wyobrażam sobie analizę takiego kodu potem.

0

Ja tylko spytałem o opinię, nie chciałem niczego złego :D W każdym razie dziękuję za każdą odpowiedź! Myślę, że dyskusję można zakończyć.

1 użytkowników online, w tym zalogowanych: 0, gości: 1, botów: 0