@jarekr000000: Nic Ci nie wmawiam. Zacząłem rozpatrywać przypadek, gdzie można wydzielić fragment większej metody do mniejszej metody tylko dla czytelności, co oznacza, że ta mniejsza metoda nigdy nie była testowana (bo wcześniej była częścią większej metody). Ty powiedziałeś, że takie wydzielenie ułatwia testowanie, co nie odnosi się do mojego przypadku, w którym takie testowanie w ogóle nie powinno mieć miejsca. Co więcej, kłóci się to z konsensusem, że bebechów w taki sposób testować się nie powinno. Jeżeli teraz powiemy, że wydzielanie metod pomaga w testowaniu, to jest to dyskusja o dwóch różnych sprawach.
No ale żeby ta rozmowa była jakaś konkretniejsza, to lepiej przeanalizować kod. Weźmy funkcję oczekującą trójmianu z pierwiastkami rzeczywistymi, zwracającą informację, czy co najmniej jeden pierwiastek jest liczbą dodatnią. Kod w javo/siszarpo/javascripto-podobnej notacji, rzucanie wyjątku jest świadome, żeby pokazać kilka dodatkowych rzeczy:
Wersja 1:
public bool hasPositiveRoot(double a, double b, double c){
double delta = b*b - 4*a*c;
if(delta < 0){
throw new Exception("Polynomial has no real roots");
}
double x1 = (-b + Math.sqrt(delta))/(2*a);
double x2 = (-b - Math.sqrt(delta))/(2*a);
return x1 > 0 || x2 > 0;
}
Wersja 2:
public bool hasPositiveRoot(double a, double b, double c){
double delta = calculateDelta(a, b, c);
validateRealRoots();
Pair roots = calculateRoots(a, b, delta);
return roots.Item1 > 0 || roots.Item2 > 0;
}
private double calculateDelta(double a, double b, double c){
return b*b - 4*a*c;
}
private void validateRealRoots(double delta){
if(delta < 0){
throw new Exception("Polynomial has no real roots");
}
}
private Pair<double, double> calculateRoots(double a, double b, double delta){
double x1 = (-b + Math.sqrt(delta))/(2*a);
double x2 = (-b - Math.sqrt(delta))/(2*a);
return Pair.of(x1, x2);
}
Wersja 3:
public bool hasPositiveRoot(double a, double b, double c){
private double calculateDelta(double a, double b, double c){
return b*b - 4*a*c;
}
private void validateRealRoots(double delta){
if(delta < 0){
throw new Exception("Polynomial has no real roots");
}
}
private Pair<double, double> calculateRoots(double a, double b, double delta){
double x1 = (-b + Math.sqrt(delta))/(2*a);
double x2 = (-b - Math.sqrt(delta))/(2*a);
return Pair.of(x1, x2);
}
double delta = calculateDelta(a, b, c);
validateRealRoots();
Pair roots = calculateRoots(a, b, delta);
return roots.Item1 > 0 || roots.Item2 > 0;
}
Wersja 4:
public bool hasPositiveRoot(double a, double b, double c){
private double calculateDelta(){
return b*b - 4*a*c;
}
private void validateRealRoots(){
if(delta < 0){
throw new Exception("Polynomial has no real roots");
}
}
private Pair<double, double> calculateRoots(){
double x1 = (-b + Math.sqrt(delta))/(2*a);
double x2 = (-b - Math.sqrt(delta))/(2*a);
return Pair.of(x1, x2);
}
double delta = calculateDelta(a, b, c);
validateRealRoots();
Pair roots = calculateRoots(a, b, delta);
return roots.Item1 > 0 || roots.Item2 > 0;
}
Wersja 5:
public bool hasPositiveRoot(double a, double b, double c){
Func<double, double, double, double> calculateDelta = (a, b, c) => b*b - 4*a*c;
Action<double> validateRealRoots = delta => { if(delta < 0) throw new Exception("Polynomial has no real roots"); };
Func<double, double, double, Pair<double, double>> calculateRoots = (a, b, delta) => {
double x1 = (-b + Math.sqrt(delta))/(2*a);
double x2 = (-b - Math.sqrt(delta))/(2*a);
return Pair.of(x1, x2);
};
double delta = calculateDelta(a, b, c);
validateRealRoots();
Pair roots = calculateRoots(a, b, delta);
return roots.Item1 > 0 || roots.Item2 > 0;
}
Wersja 6:
public bool hasPositiveRoot(double a, double b, double c){
Func<double> calculateDelta = () => b*b - 4*a*c;
Action validateRealRoots = () => { if(delta < 0) throw new Exception("Polynomial has no real roots"); };
Func<Pair<double, double>> calculateRoots = () => {
double x1 = (-b + Math.sqrt(delta))/(2*a);
double x2 = (-b - Math.sqrt(delta))/(2*a);
return Pair.of(x1, x2);
};
double delta = calculateDelta(a, b, c);
validateRealRoots();
Pair roots = calculateRoots(a, b, delta);
return roots.Item1 > 0 || roots.Item2 > 0;
}
Wersja 7:
public bool hasPositiveRoot(double a, double b, double c){
// Calculate delta
double delta = b*b - 4*a*c;
// Validate real roots
if(delta < 0){
throw new Exception("Polynomial has no real roots");
}
// Calculate roots
double x1 = (-b + Math.sqrt(delta))/(2*a);
double x2 = (-b - Math.sqrt(delta))/(2*a);
return x1 > 0 || x2 > 0;
}
I teraz tak:
- Wersja 1 - Nic specjalnego, kilka linijek kodu, ale psuje SRP (bo liczy pierwiastki i porównuje).
- Wersja 2 - Wydzielone mniejsze metody, których nie powinniśmy testować. Kod jest dłuższy, nie może zostać w pełni zinline'owany (bo rzucamy wyjątek, co psuje stos), więc teoretycznie może być wyższy koszt. Dygresja - nawet dodanie try + finally do metody (z pustymi blokami) może zwiększyć koszt wykonania znacząco. Zauważmy też, że musimy przepychać parametry między metodami.
- Wersja 3 - To samo, co wersja 2, ale tym razem funkcje te są wewnętrzne.
- Wersja 4 - Jak wersja 3, ale tym razem parametry łapiemy przez domknięcie.
- Wersja 5 - Jak wersja 3, ale tym razem zamiast metod zagnieżdżonych mamy nazwane lambdy.
- Wersja 6 - Jak wersja 5, ale tym razem zamiast parametrów łapiemy przez domknięcia.
- Wersja 7 - Jak wersja 1, ale bloki "nazwane" komentarzami (jak ktoś ma w języku regiony czy inne przydasie, to może ich użyć zamiast komentarzy).
I teraz pytania:
- Dlaczego twierdzimy, że wersje 2-6 (z wydzielonymi metodami) są czytelniejsze od wersji 1/7? Nie pytam o parametry, tylko o czytelność.
- Co lepsze, osobne metody na tym samym poziomie (wersja 2), metody zagnieżdżone (3-4), czy może lambdy (5-6)? I dlaczego?
- Lepiej przekazywać parametry w tę i nazad (2, 3, 5), czy łapać przez domknięcie (4, 6)? I dlaczego?
- Dlaczego ignorujemy spadek wydajności wersji 2-6? To jest mierzalny spadek, który zależy od platformy i rodzaju kodu, czasami inline będzie ograniczony przez długość metody, czasami przez wyjątki, czasami przez rodzaje parametrów (ref, out, wskaźniki itp), czasami po prostu kompilator nie będzie inline'ował, bo robi szybką kompilację.
- Czy jeżeli wersja 7 jest czytelna, to czy dalej chcemy ryzykować utratę wydajności przez użycie wersji 2-6? Jeżeli tak, to dlaczego?
Dla łatwości pominąłem wersję, gdzie metody dodatkowo są async, ale jeżeli byśmy takie mieli, to pytanie ekstra: dlaczego ignorujemy koszt maszyn stanowych?