EmailSession in Java Webapp

I have written an EmailSession class to connect and get (unread) mail messages. It is all working, but I would like to see your opinions on any enhancements or improvements, as I understood there are lots of bad examples on the Internet and I copy-pasted most of it together.

One issue I already noticed:

• If there is no internet connection it throws a java.net.UnknownHostException instead of handling the issue gracefully.

```
import java.util.*;
import java.util.logging.*;
import javax.mail.*;

public class EmailSession {

private Session session;
private Store store;
private Folder inbox;
private int messageCount;
private int unreadMessageCount;
private List unreadMessages;

public EmailSession() {
Properties props = new Properties();
props.setProperty("mail.store.protocol", MyProps.getProperty("mail.store.protocol"));
try {
session = Session.getInstance(props, null);
store = session.getStore();
store.connect(MyProps.getProperty("mail.host"), MyProps.getProperty("mail.user"), MyProps.getProperty("mail.password"));
inbox = store.getFolder(MyProps.getProperty("mail.foldername"));
inbox.open(Folder.READ_ONLY);
messageCount = inbox.getMessageCount();
List messages;
if (messageCount > 100) {
Logger.getLogger(EmailSession.class.getName()).log(Level.WARNING,
"More than 100 messages");
messages = Arrays.asList(inbox.getMessages(1, 100));
} else {
messages = Arrays.asList(inbox.getMessages());
}
unreadMessages = new ArrayList<>();
unreadMessageCount = 0;
for (Message message : messages) {
boolean isUnreadMessage = true;
for (Flags.Flag flag : message.getFlags().getSystemFlags()) {
if (flag == Flags.Flag.SEEN) {
isUnr

Pointless member variables

session could be a local variable in the constructor. You use it once to get store and you don't reference it again.

unreadMessageCount is pointless, because you could replace it with unreadMessages.size() instead.

In the constructor, you assign inbox, but you also close it. I'm not really familiar with how javax.mail works, but usually, after you close something it tends to become unusable. If that's the case, then the inbox member is pointless.

Pointless member variables make your class design confusing, hard to read, and potentially buggy. Try to be minimalistic, and eliminate anything from your class that it doesn't really need.

Extract logger

The logging in your code can be simplified if you extract the Logger to a constant:

private static final Logger LOGGER = Logger.getLogger(EmailSession.class.getName());

// ...
LOGGER.warning("More than 100 messages");
// ...
LOGGER.log(Level.SEVERE, null, ex);

Extract methods

The constructor is too long. It's hard to read large chunks of code. You could extract some of the functionality there to private methods to make it more readable:

private List getMessages(Folder inbox, int messageCount) throws MessagingException {
    if (messageCount > 100) {
        LOGGER.warning("More than 100 messages");
        return Arrays.asList(inbox.getMessages(1, 100));
    }
    return Arrays.asList(inbox.getMessages());
}

private List getUnreadMessages(List messages) throws MessagingException {
    List unreadMessages = new ArrayList<>();
    for (Message message : messages) {
        boolean isUnreadMessage = true;
        for (Flags.Flag flag : message.getFlags().getSystemFlags()) {
            if (flag == Flags.Flag.SEEN) {
                isUnreadMessage = false;
                break;
            }
        }
        if (isUnreadMessage) {
            unreadMessages.add(message);
        }
    }
    return unreadMessages;
}

With these helper methods and the other suggestions above, you could simplify the constructor:

public EmailSession() {
    Properties props = new Properties();
    props.setProperty("mail.store.protocol", MyProps.getProperty("mail.store.protocol"));
    Session session = Session.getInstance(props, null);
    try {
        store = session.getStore();
        store.connect(MyProps.getProperty("mail.host"), MyProps.getProperty("mail.user"), MyProps.getProperty("mail.password"));
        inbox = store.getFolder(MyProps.getProperty("mail.foldername"));
        inbox.open(Folder.READ_ONLY);
        messageCount = inbox.getMessageCount();
        List messages = getMessages(inbox, messageCount);
        inbox.close(true);
        unreadMessages = getUnreadMessages(messages);
    } catch (NoSuchProviderException ex) {
        LOGGER.log(Level.SEVERE, null, ex);
    } catch (MessagingException ex) {
        LOGGER.log(Level.SEVERE, null, ex);
    }
}

I also moved inbox.close higher to close it immediately when you don't need it anymore.

Avoid wildcard imports

Wildcard imports like import java.util.* are considered bad practice. Try to avoid it, it's better to clean up all of these:

import java.util.*;
import java.util.logging.*;
import javax.mail.*;

StackExchange Code Review Q#58611, answer score: 5