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;
}
}
1 Answer 1
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.
-
\$\begingroup\$ I agree that page.nextMessage() is probably not the way to go, but it's at least an interesting idea -- thanks. \$\endgroup\$Thufir– Thufir2013年04月02日 23:46:52 +00:00Commented Apr 2, 2013 at 23:46