Poprawne zastosowanie przeciążania metod

Czołem!

Grzebiąc na forum znalazłem tylko jeden wątek na ten temat, ale troszkę inny i odpowiedzi mnie nie satysfacjonowały. Uczę się Javy i właśnie bawię się biblioteką Javax mail.
Chciałbym napisać dwie metody wysyłające wiadomość, z czego jedna przyjmowałaby jeden argument więcej - listę osób do cc, pierwsza metoda wysyłała by tylko do adresatów bez cc.

Czy przeciążanie metod w tym przypadku jest dobrym pomysłem? Czy da się tu jakoś uniknąć powielania kodu?

Do głowy przychodzi mi jeszcze opcja stworzenia metody, która przyjmowałaby maksymalną ilość argumentów, a wywołując ją w argumencie ccList przekazywałbym null, a w metodzie sprawdzałbym jak poniżej czy przekazany argument jest nullem i według tego wywoływał odpowiednią metodę.
Wydaje mi się to jednak kompletnie "niepoprawne", aby null świadomie przekazywać jako argument.

Dzięki z góry za odpowiedź. Nie wiem, czy jako początkujący nie za bardzo zagłębiam się w szczegóły?

mis_barei napisał(a):

Czy przeciążanie metod w tym przypadku jest dobrym pomysłem? Czy da się tu jakoś uniknąć powielania kodu?

Przeciążanie to nic innego jak dwie osobne funkcje, które po prostu mają taką samą nazwę. Nie dają niczego dodatkowego, czego nie mógłbyś załatwić po prostu dwoma metodami.

Więc to nie jest tak że "przeciążanie metody" pozwala uniknąć powielania kodu, tylko po prostu metody pozwalają na code re-use - a to czy mają taką samą czy inną nazwę to szczegół.

W skrócie tak - osobne funkcje z osobnymi sygnaturami to dobry pomysł. To czy mają mieć taką samą czy inną nazwę to kwestia preferencji czy gustu IMO.

Do głowy przychodzi mi jeszcze opcja stworzenia metody, która przyjmowałaby maksymalną ilość argumentów, a wywołując ją w argumencie ccList przekazywałbym null, a w metodzie sprawdzałbym jak poniżej czy przekazany argument jest nullem i według tego wywoływał odpowiednią metodę.

To na pewno jest gorszy sposób.

Moim zdaniem nie ma większego znaczenia. Ja skłaniałbym się ku wersji dwóch osobnych metod. To czy będziesz tu coś nadpisywać to nie ma już moim zdaniem takiego dużego znaczenia.

W drugim przypadku mógłbyś przekazywać wszystkie argumenty a ccList opakować w Optional ale wydaje mi się przekombinowane. O powielaniu kodu możemy mówić gdybyś tych metod miał faktycznie z 10. Tutaj masz dwie i dodatkowy argument ccList nie jest tam zbędny.

Ja bym zostawił te 2 metody, z czego obydwie wołałyby tą samą prywatną metodę, z tym że jedna by ją wołała z nullem jako ccList (albo może lepiej wrzucić tam pustą listę, a nie nulla?), a sama obsługa tego nulla byłaby właśnie w tej prywatnej metodzie. Dzięki temu masz przejrzyste publiczne API, a szczegóły implementacyjne w prywatnej metodzie.

MckMaciek napisał(a):

Ja bym zostawił te 2 metody, z czego obydwie wołałyby tą samą prywatną metodę, z tym że jedna by ją wołała z nullem jako ccList (albo może lepiej wrzucić tam pustą listę, a nie nulla?), a sama obsługa tego nulla byłaby właśnie w tej prywatnej metodzie. Dzięki temu masz przejrzyste publiczne API, a szczegóły implementacyjne w prywatnej metodzie.

Ale po co używać tej patologii zwanej nullami i jakiegoś ifowania? Niech stworzy prywatną metodę przyjmującą jeden parametr Message message, a 2 przeciężone metody niech odpowiadają za zbudowanie message z lub bez ccList

Nie wklejaj kodu jako obrazki.
A jak jest przeciążona metoda prepareMessage? Nie możesz tam przekazać nulla czy pustej listy?
Moim zdaniem możesz przeciążyć jeśli chcesz i po prostu wywołać jedną metodę z drugiej:

public void sendMessage(String messageText, String subject, String recipientsList, String ccList) {
  try {
    Session session = createSession();
    Message message = prepareMessage(session, messageText, subject, recipientsList, ccList);
    Transport.send(message);
    copyMessageToSentItemsFoIder(message);
  catch (Exception e) {
    throw new IllegalStateException(e);
  }
}

public void sendMessage(String messageText, String subject, String recipientsList) {
  sendMessage(messageText, subject, recipientsList, "");
}

kod skopiowany z obrazka powertoolsami więc mogą być błędy.
Raczej unikaj powtarzania większości kodu w dwóch metodach bo jak będziesz chciał coś zmienić to musisz to robić w wielu miejscach i łatwo o błędy.
Jeśli nie chcesz ifologii w środku i nullów to wydziel wspólną logikę i na przykład tak jak wyżej sugerował @Grzyboo prepareMessage wywołuj na zewnątrz a metoda niech tylko zajmie się wysyłaniem. Będziesz miał bardziej elastyczny kod i separację logiki. Co w przypadku jak w przyszłości będziesz chciał wysłać maila z załącznikiem? A potem z załącznikiem i CC i z załącznikiem bez CC i może jeszcze bcc albo bez, będziesz tworzył 50 metod na wszystkie warianty?

obscurity napisał(a):.

Jeśli nie chcesz ifologii w środku i nullów to wydziel wspólną logikę i na przykład tak jak wyżej sugerował @Grzyboo prepareMessage wywołuj na zewnątrz a metoda niech tylko zajmie się wysyłaniem.

To jest bardzo dobra rada.

Będziesz miał bardziej elastyczny kod i separację logiki.

A to z kolei bardzo wartościowe rzeczy, z pewnością należałoby to dodać.

Co w przypadku jak w przyszłości będziesz chciał wysłać maila z załącznikiem? A potem z załącznikiem i CC i z załącznikiem bez CC i może jeszcze bcc albo bez, będziesz tworzył 50 metod na wszystkie warianty?

Ale to jest niemądre gdybanie. YAGNI.

@obscurity: Zamiast tego należałoby to ubrać w słowa "Rozszerzalność kodu ma wiele zalet - jednym ze sposobów na wytwarzanie aplikacji szybko, jest nie stawianie sobie kłód pod nogi, a taka ifologia w nierozszerzalnych funkcjach to jedna z takich kłód. Jeśli przypadkiem miałbyś dodać kolejną funkcjonalność do swojej funkcji (ciężko to przewidzieć, ale może to być np dodanie kolejnych adresatów, dodanie CC albo dodanie zalącznika - albo cokolwiek innego, jakiekolwiek inne wymaganie, którego teraz nie przewidzisz). Dodatkowo, sama praca z taką funkcją jest dużo szybsza, gdybyś chciał ją zrefaktorować, przenieść, reużyć".

mis_barei napisał(a):

Do głowy przychodzi mi jeszcze opcja stworzenia metody, która przyjmowałaby maksymalną ilość argumentów, a wywołując ją w argumencie ccList przekazywałbym null, a w metodzie sprawdzałbym jak poniżej czy przekazany argument jest nullem i według tego wywoływał odpowiednią metodę.
Wydaje mi się to jednak kompletnie "niepoprawne", aby null świadomie przekazywać jako argument.

W java to jest bardzo niepoprawne podejście. Sygnatura metody powinna jasno wskazywać które parametry są wymagane, a które opcjonalne. W Java nie ma takiej możliwości, więc należy unikać. W językach, które mają parametry nazwane + null safety IMO jest to dopuszczalny pomysł:

fun sendMessage(subject:String, to: List<String>, cc: List?<String>)

W Java zrobiłbym przeciążenie "teleskopowe", czyli implementujesz jedną metodę, tę z największą liczbą parametrów, do niej dorabiasz metody z domyślnymi wartościami tych parametrów jak w przykładzie @obscurity

Jeżeli pojawi się potrzeba dodania kolejnego parametru, masz o tyle prosto, że dodajesz ten parametr + dodajesz kolejne przeciążenie tej metody, czyli zmiany będą ograniczone do jednego miejsca.

public void sendMessage(String messageText, String subject, String recipientsList, String ccList) {
  //jakaś tam implementacja
}

public void sendMessage(String messageText, String subject, String recipientsList) {
  sendMessage(messageText, subject, recipientsList, "");
}

i po refaktoryzacji z dodaniem parametru z załącznikami:

public void sendMessage(String messageText, String subject, String recipientsList, String ccList, Attachement attachment) {
  //jakaś tam implementacja
}

public void sendMessage(String messageText, String subject, String recipientsList, String ccList) {
  sendMessage(messageText, subject, recipientsList, ccList, null);
}

public void sendMessage(String messageText, String subject, String recipientsList) {
  sendMessage(messageText, subject, recipientsList, "");
}