Cięzkie, nocne rozkminy

1:

boolean isValid (String msg) {
    // ...
}

boolean sendMail (String msg, MailAddress from, MailAddress to) {
    Service service = Service.builder().setFrom(from).setTo(to).build();
    service.sendMail(msg);
}

boolean sendMails (List<String> msg, MailAddress from, MailAddress to) {
    for (String m : msg) {
        if ( isValid(msg) ) {
            sendMail(msg, from, to);
        }
    }
}

2:

boolean isValid (String msg) {
    // ...
}

boolean sendMail (String msg, MailAddress from, MailAddress to) {
    Service service = Service.builder().setFrom(from).setTo(to).build();
    if ( isValid(msg) {
        service.sendMail(msg);
    }
}

boolean sendMails (List<String> msg, MailAddress from, MailAddress to) {
    for (String m : msg) {
        sendMail(msg, from, to);
    }
}

Ja bym się kierował prostą zasadą, że im wcześniej następuje walidacja tym lepiej, więc raczej opcja (1). Jeśli msg pochodzi z zewnątrz (web input, plik, etc.), to może warto taką walidację umieścić w adapterze, który te dane odbiera i dalej operować już na domenowym obiekcie (np. MailBody), co do którego mielibyśmy pewność, że jest poprawny. Jeśli na siłę doszukiwać się czynnika wydajnościowego, to walidacja wcześniej pozwala oszczędzić parę niepotrzebnych operacji - zauważ, że w 2 konstruujesz service za każdym razem, bez względu na rezultat walidacji. W prostym przypadku to nie problem bo i tak można taki kod przenieść do wnętrza if, ale przy dużym projekcie, gdzie stack trace ciągnie się kilometrami, im wcześniej ominiemy niepotrzebne wywołania, tym mniej niespodzianek.

Moim zdaniem opcja 3:

• Powinieneś mieć obiekt Mail który zawiera te 3 pola
• Walidacja powinna się dziać kiedy tworzysz ten obiekt, a powinieneś go tworzyć praktycznie od razu
• Nie do końca rozumiem ideę tego Buildera do serwisu tutaj, bo akurat samo wysyłanie maila bardziej pasuje na akcje jakiegoś bezstanowego serwisu niż na operacje na obiekcie domenowym

W opcji 1 jesteś w stanie wysłać niezwalidowanego maila. @Shalom ma fajny pomysł.

Charles_Ray napisał(a):

W opcji 1 jesteś w stanie wysłać niezwalidowanego maila. @Shalom ma fajny pomysł.

Oprócz tego w opcji nr 2 łatwiej będzie to przerobić na lambdę i streama

boolean sendMails(List<String> msgs, MailAddress from, MailAddress to) {
    msgs.foreach(m -> sendMail(m, from, to));
}

Druga funcja nie ma większego sensu (nawet jeśli robiłaby walidację).

Ona miałaby sens, gdyby chodziło o podkreślenie, że w takim mnogim wariancie zadanie zostanie przeprowadzone optymalniej.

Jeśli tego nie robi to ta funkcja będzie mylić inne osoby. Poza tym jeśli ktoś zrobiłby kolejną pętlę, ale w niej dodatkowo dorobiłby logowanie to w czym taka operacja byłaby gorsza/lepsza od Twojej pętli? Niczym - co wskazuje na to, że robisz coś na zaś, zbędny nadmiar.

Jeśli ze względu na różny kontekst maile będziecie inaczej wysyłać to rób osobne klasy powiązane z wybranym kontektsem, a podstawowa klasa niech nadal zostanie z prostym prymitywem do wysyłania jednej wiadomości.

Zyskasz wtedy prostsze api i podział, który pozwoli Ci łatwiej wycofowywać się ze słabych pomysłów.

Nie zapędzajmy się z nowymi pomysłami, bo tak na prawdę ja to klepię w innym języku, bez obiektów i strimów, hehe.