1
\$\begingroup\$

I know that Swing isn't true MVC:

Using this modified MVC helps to more completely decouple the model from the view.

Leaving aside the veracity of the above claim, the problem I run into is that I can't seem to just drop a Page into a JList, but have to first extract a List (data) from the Page object. This just seems like a bad idea. Does this break encapsulation?

What's a better approach to handling the nextPage method, invoked by clicking the next button?

package net.bounceme.dur.nntp.swing;

import java.awt.BorderLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.KeyEvent;
import java.awt.event.MouseEvent;
import java.util.List;
import java.util.logging.Logger;
import javax.mail.Message;
import javax.mail.MessagingException;
import javax.swing.DefaultListModel;
import javax.swing.JButton;
import javax.swing.JList;
import javax.swing.JPanel;
import javax.swing.JScrollPane;
import net.bounceme.dur.nntp.gnu.PageMetaData;
import net.bounceme.dur.nntp.gnu.Page;
import net.bounceme.dur.nntp.gnu.Usenet;

public class ArticlesPanel extends JPanel {

    private static final Logger LOG = Logger.getLogger(ArticlesPanel.class.getName());
    private static final long serialVersionUID = 1L;
    private JList<String> jList = new JList<>();
    private JScrollPane scrollPane = new JScrollPane();
    private DefaultListModel<String> defaultListModel;
    private JButton next = new JButton("next");
    private Page page;
    private Usenet usenetConnection = Usenet.INSTANCE;  //ensures correct connection
    private PageMetaData pageMetaData = new PageMetaData();

    public ArticlesPanel() {
        nextPage(null);
        nextPage(null);  //only because default page starts at zero
        initComponents();
    }

    @SuppressWarnings("unchecked")
    private void initComponents() {
        setLayout(new java.awt.BorderLayout());
        next.addActionListener(new ActionListener() {

            public void actionPerformed(ActionEvent e) {
                nextPage(e);
            }
        });

        jList.setModel(defaultListModel);
        jList.setSelectionMode(javax.swing.ListSelectionModel.SINGLE_SELECTION);
        jList.addMouseListener(new java.awt.event.MouseAdapter() {

            public void mouseReleased(java.awt.event.MouseEvent evt) {
                mouseReleases(evt);
            }
        });
        jList.addKeyListener(new java.awt.event.KeyAdapter() {

            public void keyReleased(java.awt.event.KeyEvent evt) {
                keyReleases(evt);
            }
        });

        scrollPane.setViewportView(jList);
        add(scrollPane, BorderLayout.CENTER);
        add(next, BorderLayout.SOUTH);

        this.setSize(300, 100);
        scrollPane.setVisible(true);
        this.setVisible(true);
    }

    private void keyReleases(KeyEvent evt) {
        itemSelected();
    }

    private void mouseReleases(MouseEvent evt) {
        itemSelected();
    }

    private void itemSelected() {
        LOG.info("selected\t\t" + jList.getSelectedValue());
    }

    private void nextPage(ActionEvent e) {
        page = usenetConnection.getPage(pageMetaData);  //first time, default
        pageMetaData = new PageMetaData(page.getPageMetaData(), true); //get next is true
        List<Message> messages = page.getMessages(); //breaks MVC?
        defaultListModel = new DefaultListModel<>();  //clear or new?
        for (Message m : messages) {
            try {
                defaultListModel.addElement(m.getSubject());
            } catch (MessagingException ex) {
                LOG.warning("bad message\n" + m.toString() + "\n" + ex);
            }
        }
        jList.setModel(defaultListModel);
        LOG.fine(page.toString());
    }
}

While it's messy, that's not problem I'm facing. Rather, it's data encapsulation, I believe. I want Page to hide as much data as possible, yet that seems impossible.

Page class:

package net.bounceme.dur.nntp.gnu;

import gnu.mail.providers.nntp.GroupMetaData;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
import java.util.List;
import java.util.logging.Logger;
import javax.mail.Header;
import javax.mail.Message;
import javax.mail.MessagingException;

public class Page {

    private final static Logger LOG = Logger.getLogger(Page.class.getName());
    private List<Message> messages = new ArrayList<>();
     private PageMetaData pageMetaData = new PageMetaData();

    public Page() throws Exception {
        GroupMetaData gmd = new GroupMetaData();
        pageMetaData = new PageMetaData(gmd);
    }

    public Page(GroupMetaData gmd) throws Exception {
        pageMetaData = new PageMetaData(gmd);
    }

    public Page(PageMetaData pmd) throws Exception {
        this.pageMetaData = pmd;
    }

    public Page(PageMetaData pmd, List<Message> messages) throws MessagingException {
        this.pageMetaData = pmd;
        this.messages = messages;
    }


    public List<Message> getMessages() {
        return Collections.unmodifiableList(messages);
    }

    private String printXref() throws MessagingException {
        LOG.fine("starting xref printing...\t" + messages.size());
        StringBuilder sb = new StringBuilder();
        String s = null;
        String headerString = null;
        String subString = null;
        sb.append("messages to follow\n");
        for (Message message : messages) {

            LOG.fine(message.getSubject());

            int i = messages.indexOf(message);
            Enumeration headers = null;

            headers = message.getAllHeaders();

            while (headers.hasMoreElements()) {
                Object o = headers.nextElement();
                Header header = (Header) o;
                if ("Xref".equals(header.getName())) {
                    headerString = header.getValue();
                    int index = headerString.indexOf(":");
                    subString = headerString.substring(index + 1);
                    int xref = Integer.parseInt(subString);
                    s = "\n" + i + "\t\t" + xref;
                    sb.append(s);
                    s = message.getSubject();
                    sb.append("\t").append(s);
                }
            }
            LOG.fine("\n\n\n**********************\n\n\n");
        }
        LOG.fine(sb.toString());
        return sb.toString();
    }

    public String toString() {
        String s = "\n---new page---\n" + getPageMetaData().toString() + "\n";
        try {
            s = s + printXref();
        } catch (MessagingException ex) {
            ex.printStackTrace();
        }
        return s;
    }

    public PageMetaData getPageMetaData() {
        return pageMetaData;
    }

    public void setPageMetaData(PageMetaData pageMetaData) {
        this.pageMetaData = pageMetaData;
    }
}

Full project

\$\endgroup\$
1
\$\begingroup\$

Extract the busy-work code of transforming the List<Message> to DefaultListModel into another method.

From what I'm seeing I would think about the concepts you want to express in your code vis-a-vis some technical adherence to "encapsulation". If you want Page users to have no concept of an imbedded Message in a Page fine, but as it stands I'm not thinking message is inadequately encapsulated. Alternatively if there are some Message public getters that you simply do not want Page users to access, then that's OK rational too.

In this case then the alternative is, in the Page class, a public public List<String> Page.getSubjects() - and likewise for all Message properties you want public. If, as far as Page users are concerned, the message's subject is the message, then rename the methods: Page.getMessages(); but it still returns a List<string>.

You can take that one step further by having Page implement iterator methods so the calling code looks like page.nextMessage(), page.hasNextMessage(), etc. and whatever. But again, the caller has to iterate. This doesn't feel like much bang for get buck to me, although it seems to fanatically adhere to the law of demeter.

\$\endgroup\$
  • \$\begingroup\$ I agree that page.nextMessage() is probably not the way to go, but it's at least an interesting idea -- thanks. \$\endgroup\$ – Thufir Apr 2 '13 at 23:46

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.