patternjavaMinor
Overloading email sender utility class methods take single or array arguments
Viewed 0 times
argumentsoverloadingemailarraytakesenderutilitymethodssingleclass
Problem
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
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?
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?
Solution
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
Now that's only a stub but there's a lot you can add to it. You can even add a
In the end however, your method would just be this:
Which surely is easier to grasp than all those
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 recepients;
private final List cc;
private final List 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!Code Snippets
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;
}
...
}public void sendEmail(EmailOptions email) {
...
}Context
StackExchange Code Review Q#70332, answer score: 3
Revisions (0)
No revisions yet.