5
\$\begingroup\$

I'm writing a simple utility class for sending an email. I'm not sure what's the proper way to present this to a consumer. Should I force them to use an array in send()? Or should I provide an overload so they can pass a single argument?

public class SimpleSender {

    public static void send(String smtpServer, String[] to, String[] cc, String from,
                            String subject, String body, boolean HTML, String fullFilePath) {
        SendEmail(smtpServer, to, cc, from, subject, body, HTML, fullFilePath);
    }
    public static void send(String smtpServer, String[] to, String[] cc, String from,
                            String subject, String body, boolean HTML) {
        SendEmail(smtpServer, to, cc, from, subject, body, HTML, null);
    }

    // Should I provide this overload and similar ones?
    public static void send(String smtpServer, String to, String[] cc, String from,
                            String subject, String body, boolean HTML) {
        SendEmail(smtpServer, new String[] {to}, cc, from, subject, body, HTML, null);
    }
    private static void SendEmail(String smtpServer, String[] to, String[] cc, String from, String subject, String body, boolean HTML, String fullFilePath) {
     .... Send email
    }
}

I know it's a bit of overkill for just a simple email utility, but in general, what's a good practice for these situations?

\$\endgroup\$
3
\$\begingroup\$

Your code has a particular Code Smell:

Too many parameters: a long list of parameters is hard to read, and makes calling and testing the function complicated. It may indicate that the purpose of the function is ill-conceived and that the code should be refactored so responsibility is assigned in a more clean-cut way.

One way to fix this code smell is by extracting the parameters to a EmailOptions class (or EmailDraft or something).

public class EmailOptions {
     private final List<String> recepients;
     private final List<String> cc;
     private final List<String> bcc;

     public void setHTML(boolean html) {
         this.html = html;
     }

     ...
}

Now that's only a stub but there's a lot you can add to it. You can even add a addReceipient(String email) method (also include some kind of e-mail validation if you feel like it).

In the end however, your method would just be this:

public void sendEmail(EmailOptions email) {
    ...
}

Which surely is easier to grasp than all those String parameters!

\$\endgroup\$
2
\$\begingroup\$

The best way to deal with compulsory and optional parameters is using the Builder pattern.The problem with overloaded functions is that they are confusing and user of the class or even you can make mistakes while calling this method.

Instead of that you can have builder that allows to construct Emails fluently.

class EmailSender{

private final String body;
private final String recipient;
private final List<String> to;
private final List<String> ccs;

public  EmailSender(EMailBuilder mailBuilder){
    this.body = mailBuilder.body;
    this.to = mailBuilder.to;
    this.recipient = mailBuilder.recipient;
    this.ccs = mailBuilder.ccs;

}
public void send(){
    //
}
public  static  class EMailBuilder{

    private final String body;
    private final String recipient;
    private final List<String> to;
    private final List<String> ccs;

    public  EMailBuilder(String body,String recipient,String to){
        this.recipient = recipient;
        this.body = body;
        this.to = new ArrayList<>();
        this.to.add(to);
        this.ccs = new ArrayList<>();

    }

    public EMailBuilder addTo(String to){
        this.to.add(to);
        return this;
    }
    public EMailBuilder addCc(String cc){
        this.ccs.add(cc);
        return this;
    }

    public EmailSender build(){
        EmailSender sender = new EmailSender(this);
        return sender;
    }

 }
}

Something like allows you build things easily, for example:

  EmailSender emailSender = new EmailSender.EMailBuilder("This is an email","me","you")
            .addTo("the Other Chap").addCc("the boss")
            .build();
  emailSender.send();

The problem with your code is it hard to get right and validate. Is it allowed to send an email with a to ? It shouldn't, and hence you need to length of the array.

  if(to.length==0){
   // invalid 
   }

The beauty about the builder pattern is that it tells what's compulsory and what's optional.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.