Bulk Email notifications in Java?

Question

I have written a service which uses the Java Mail Api to send bulk email notifications to the recipients. I feel that the way I implemented the solution is not really scalable when the number of recipients increases. I would like to know if there is a better way to make my solution more efficient.

This is what I tried so far

@Component
public class EmailService {
 public String sendEmail(String [] to,String from, String subject, String msg) throws MessagingException {
 String status ="099";
 final String password ="********";
 final String fromsender = from;
 Properties props = new Properties(); 
 props.setProperty("mail.transport.protocol", "smtp"); 
 props.setProperty("mail.host", "smtp.gmail.com"); 
 props.put("mail.smtp.auth", "true"); 
 props.put("mail.smtp.port", "465"); 
 props.put("mail.debug", "true"); 
 props.put("mail.smtp.socketFactory.port", "465"); 
 props.put("mail.smtp.socketFactory.class","javax.net.ssl.SSLSocketFactory"); 
 props.put("mail.smtp.socketFactory.fallback", "false"); 
 Session session = Session.getDefaultInstance(props, 
 new javax.mail.Authenticator() {
 protected PasswordAuthentication getPasswordAuthentication() { 
 return new PasswordAuthentication(fromsender,password); 
 } 
 }); 
 //session.setDebug(true); 
 Transport transport;
 try {
 transport = session.getTransport();
 InternetAddress addressFrom = new InternetAddress(fromsender); 
 MimeMessage message = new MimeMessage(session); 
 message.setSender(addressFrom); 
 message.setSubject(subject); 
 message.setContent(msg, "text/plain"); 
 //message.addRecipient(Message.RecipientType.TO, new InternetAddress(to)); 
 for(int i =0;i<to.length;i++){
 message.addRecipient(Message.RecipientType.TO, new InternetAddress(to[i])); 
 }
 transport.connect(); 
 Transport.send(message); 
 status ="000";
 transport.close();
 } catch (NoSuchProviderException e) {
 // TODO Auto-generated catch block
 e.printStackTrace();
 } 
 return status;
 }

• Mario Santini: If the list of recipients is very huge, you should consider to configure an e-mail alias or a mailing list address, and left the task to the mail server.
• Mario Santini: Another consideration about the recipients list is when the list is very long, it is not so useful for the recipient to have such e-mai: he/she can replay to all! Maybe over certain number, you can consider to send the e-mail individually.
• Toby Speight: Welcome to Code Review! You'll receive better reviews if you show a complete example. For example, I recommend that you show the necessary import lines, and a main() that shows how to call your function. It's not mandatory, but it really helps!

Answer 1

Indentation

Please please please please automatically indent your code. You're doing all readers (and yourself) a disservice by not following indentation conventions and enforcing them through your IDE.

Abstraction

This code can be much cleaner by simply extracting responsibilities into methods, especially those that encapsulate setup. Let's get the code into a final state as follows:

public String sendEmail(String[] to, String from, String subject, String msg) throws MessagingException {
 String status = "099";
 final Session session = buildMailSession(from, password);
 final MimeMessage message = buildMailMessage(session, from, to, subject, msg);
 final Transport transport;
 try {
 transport = session.getTransport();
 transport.connect();
 Transport.send(message);
 status = "000";
 } catch (NoSuchProviderException e) {
 e.printStackTrace();
 } finally {
 if (transport != null) {
 transport.close();
 }
 }
 return status;
}

This is signifcantly easier to comprehend and understand. It even makes clear, what status is. You should very much consider extracting those into named constants.

This is a good moment to also talk about documentation.

When extracting methods, that is a huge opportunity to include some "free" documentation through the method name. In case that is not sufficient, one should employ *javadoc" to clarify what the method does exactly:

/**
 * Sends an email to all the recipients in <tt>to</tt>
 * 
 * @return {@link #SUCCESS_STATUS} if sending was successful,
 * {@link FAIL_STATUS} otherwise
 */
public String sendEmail(..);

Signature changes

Instead of returning magic Strings, it would be much cleaner to return an enum value like so:

public SendResult sendEmail(...) {
 SendResult status = SendResult.SEND_FAIL;
 // ...
 status = SendResult.SEND_SUCCESS;
 // ...
 return status;
}

Now if we use the fact that I introduced a finally block, which is executed regardless of how the corresponding try-block is left we can simplify this to:

public SendResult sendEmail(...) {
 // ...
 Transport.send(message);
 return SendResult.SEND_SUCCESS;
 } catch (..) {
 // ..
 return SendResult.SEND_FAIL;
}

In addition to that you should consider changing the signature to the following:

public SendResult sendEmail(String msg, String subject, String from, String... to)

This makes use of "varargs" to allow invoking it as follows:

emailService.sendEmail(message, subject, from, "[email protected]", "[email protected]");

Whether that's more useful... How would I know?