4
\$\begingroup\$

As a study project I was supposed to write an RSS reader in Java (I used Swing to make GUI) using MVC pattern.

I finished it, but it's not quite polished yet (still gotta write these javadocs and add deleting categories/channels) and there are most likely a lot of dumb things as I wrote my first line of code in Java a week ago.

I would really appreciate if you could check if I implemented MVC correctly (specifically my Controller which works as "delegate"), as I'm honestly a bit confused what should I put where and apparently there are A LOT of ways to implement the pattern.

Right now I use the Controller as a "bridge" between Model and View, but I don't know for example if, for example, elementInTreeFocusedEvent(JTree tree) should be there.

The interface (as in, error alerts, button labels and so on) is in Polish, but you should have no problem with it.

GitHub

Feeder.java

package feeder; import feeder.controller.Controller; import feeder.views.View; public class Feeder { public static void main(String[] args) { new View(new Controller()); } } 

model/News.java

package feeder.model; import java.io.Serializable; /** * <b>News</b> is a container class for a single news item. * * @since 2016-06-10 * @version 1.0 */ public class News implements Serializable { private static final long serialVersionUID = 1L; private String title; private String link; private String date; private String channel; private String description; /** * @return the title */ public String getTitle() { return title; } /** * @param title the title to set */ public void setTitle(String title) { this.title = title; } /** * @return the link */ public String getLink() { return link; } /** * @param link the link to set */ public void setLink(String link) { this.link = link; } /** * @return the date */ public String getDate() { return date; } /** * @param date the date to set */ public void setDate(String date) { this.date = date; } /** * @return the category */ public String getChannel() { return channel; } /** * @param category the category to set */ public void setChannel(String channel) { this.channel = channel; } /** * @return the description */ public String getDescription() { return description; } /** * @param description the description to set */ public void setDescription(String description) { this.description = description; } } 

model/Channel.java

package feeder.model; import java.io.IOException; import java.io.Serializable; import java.net.URL; import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.List; import com.rometools.rome.feed.module.DCModule; import com.rometools.rome.feed.module.Module; import com.rometools.rome.feed.synd.SyndEntry; import com.rometools.rome.feed.synd.SyndFeed; import com.rometools.rome.io.FeedException; import com.rometools.rome.io.SyndFeedInput; import com.rometools.rome.io.XmlReader; /** * <b>Channel</b> is a class representing an RSS feed (or RSS channel). * It's lowest in the model hierarchy and does the most important thing * which is parsing the XML file using ROME library and generating * List of News which can be then easily formatted and displayed. * * @since 2016-06-01 * @version 0.7 */ public class Channel implements Serializable { private static final long serialVersionUID = 1L; private String name; private URL url; private SyndFeed feed; private boolean isAggregated; /** * Class constructor for URL-based feeds (so the ones that user is adding). * Creates the FeedSynd which is later used to generate the content; * * @param name Name of the Channel. * @param url URL of the XML file. * @throws IOException Thrown when the URL is corrupted. * @throws FeedException Thrown when ROME couldn't create the SyndFeed. * @throws IllegalArgumentException Thrown when the URL given content isn't in XML format. */ public Channel(final String name, final String url) throws IllegalArgumentException, FeedException, IOException { this.name = name; this.url = new URL(url); feed = new SyndFeedInput().build(new XmlReader(this.url)); isAggregated = false; } /** * Class constructor for aggregated feeds (so the one for whole Category). * * @param feed SyndFeed aggregated by Category class method. * @see Category#getAggregatedFeed() */ public Channel(final SyndFeed feed) { this.feed = feed; this.name = feed.getTitle(); isAggregated = true; } /** * Cleans up all the entries from SyndFeed feed, packs each * of them to the News container class and then makes a List of them. * * @return List of News. * @throws IllegalArgumentException * @throws FeedException * @throws IOException */ public List<News> getChannelContent() throws IllegalArgumentException, FeedException, IOException { if(isAggregated == false) { feed = new SyndFeedInput().build(new XmlReader(this.url)); } List<News> content = new ArrayList<News>(); for(SyndEntry entry : feed.getEntries()) { News news = new News(); news.setTitle(entry.getTitle()); news.setLink(entry.getLink()); if(entry.getPublishedDate() != null) { SimpleDateFormat dateFormat = new SimpleDateFormat("dd.MM.yyyy, HH:mm"); news.setDate(dateFormat.format(entry.getPublishedDate())); } if(entry.getDescription() != null) { news.setDescription(entry.getDescription().getValue().trim().replaceAll("\\<.*?\\>", "")); } if(isAggregated == true) { Module dcModule = entry.getModule(DCModule.URI); news.setChannel(((DCModule)dcModule).getCreator()); } content.add(news); } return content; } /** * @return SyndFeed object. */ public SyndFeed getFeedInput() { return feed; } /** * @return {@link Channel} name. */ public String getName() { return name; } /** * @return URL as String. */ public String getUrl() { return url.toString(); } public boolean isAggregated() { return isAggregated; } } 

model/Category.java

package feeder.model; import java.io.IOException; import java.io.Serializable; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; import com.rometools.rome.feed.module.DCModule; import com.rometools.rome.feed.module.Module; import com.rometools.rome.feed.synd.SyndEntry; import com.rometools.rome.feed.synd.SyndFeed; import com.rometools.rome.feed.synd.SyndFeedImpl; import com.rometools.rome.io.FeedException; /** * <b>Category</b> represents folder/parent of Channel or multiple Channels. * Used mostly to organize Channels, generates aggregated feed. * * @since 2016-06-01 * @version 0.7 */ public class Category implements Serializable { private static final long serialVersionUID = 1L; private final String name; private Integer index = null; private Map<String, Channel> channels; /** * Constructor. Creates List object containing related {@link Channel}s. * * @param name Name of the category visible in the tree. */ public Category(String name) { channels = new HashMap<String, Channel>(); this.name = name; } /** * Simply adds {@link Channel} to the list of channels. * * @param channel Channel object to add */ public void addChannel(Channel channel) { channels.put(channel.getName(), channel); } /** * Aggregates feeds from all {@link Channel}s belonging to the {@link Category} and * sorts them by date of publication. * * @return HTML-formatted String with data like in {@link Channel#convertToHTML(SyndFeed, boolean)} * @throws FeedException * @throws IOException * @throws IllegalArgumentException */ public Channel getAggregatedFeed() throws FeedException, IllegalArgumentException, IOException { SyndFeed feed = new SyndFeedImpl(); List<SyndEntry> entries = new ArrayList<SyndEntry>(); feed.setFeedType("rss_2.0"); feed.setTitle(getName()); feed.setDescription(getName() + " - wszystkie nagłówki"); feed.setEntries(entries); for(Entry<String, Channel> channelEntry : channels.entrySet()) { Channel channel = channelEntry.getValue(); SyndFeed subFeed = channel.getFeedInput(); // adds note from which category each entry comes for (SyndEntry entry : subFeed.getEntries()) { Module dcModule = entry.getModule(DCModule.URI); ((DCModule)dcModule).setCreator(channel.getName()); entries.add(entry); } } // custom Comparator - sorts entries by date (descending) Collections.sort(entries, new Comparator<SyndEntry> () { public int compare(SyndEntry firstEntry, SyndEntry secondEntry) { return secondEntry.getPublishedDate().compareTo(firstEntry.getPublishedDate()); } }); return new Channel(feed); } public Channel getChannelContent(String name) throws IllegalArgumentException, FeedException, IOException { return channels.get(name); } /** * @return Name of the {@link Category}. */ public String getName() { return name; } /** * @return the channels */ public Map<String, Channel> getChannelsMap() { return channels; } /** * @param index */ public void setIndex(Integer index) { this.index = index; } /** * @return */ public Integer getIndex() { return index; } } 

model/Model.java

package feeder.model; import java.io.IOException; import java.io.Serializable; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Map.Entry; import javax.swing.tree.DefaultMutableTreeNode; import javax.swing.tree.DefaultTreeModel; import com.rometools.rome.io.FeedException; /** * <b>Model</b> is an element of MVC structure - using DefaultTreeModel, * DefaultMutableTreeNode and obviously Channel and Category classes it * represents business logic of this application. * * @since 2016-06-02 * @version 0.8 */ public class Model implements Serializable { private static final long serialVersionUID = 1L; private Map<String, Category> categories; private DefaultMutableTreeNode root; private DefaultTreeModel treeModel; public Model() { categories = new HashMap<String, Category>(); root = new DefaultMutableTreeNode("Root"); treeModel = new DefaultTreeModel(root); } /** * Adds a new Category to the tree. * * @param name Name of the Category as String. */ public void addNewCategory(String name) { Category category = new Category(name); DefaultMutableTreeNode node = new DefaultMutableTreeNode(name); categories.put(name, category); root.add(node); category.setIndex(root.getIndex(node)); treeModel.reload(); } /** * Creates a Channel and adds it to the Category. * Category must already exist. * * @param chanName as String * @param chanUrl as String * @param category as String * @throws IllegalArgumentException * @throws FeedException * @throws IOException */ public void addNewChannel(String channelName, String channelUrl, String categoryName) throws IllegalArgumentException, FeedException, IOException { Channel channel = new Channel(channelName, channelUrl); Category category = categories.get(categoryName); category.addChannel(channel); DefaultMutableTreeNode categoryNode = (DefaultMutableTreeNode) root.getChildAt(category.getIndex()); treeModel.insertNodeInto(new DefaultMutableTreeNode(channelName), categoryNode, categoryNode.getChildCount()); treeModel.reload(); } /** * @param category * @param channel * @return Formatted feed as String. * @throws NoSuchElementException * @throws IllegalArgumentException * @throws IOException * @throws FeedException * @see Channel#convertToHTML(com.rometools.rome.feed.synd.SyndFeed, boolean) */ public Channel getChannelFeed(String category, String channel) throws IllegalArgumentException, IOException, FeedException { return categories.get(category).getChannelContent(channel); } /** * @param Category name of the category to get aggregated feed. * @return Aggregated, formatted feed as String. * @throws FeedException * @throws IOException * @throws IllegalArgumentException * @see Category#getAggregateFeed() * @see Channel#convertToHTML(com.rometools.rome.feed.synd.SyndFeed, boolean) */ public Channel getCategoryFeed(String category) throws FeedException, IllegalArgumentException, IOException { return categories.get(category).getAggregatedFeed(); } /** * @return List of categories names. */ public String[] getCategoriesList() { List<String> categoriesList = new ArrayList<String>(); for(Entry<String, Category> category : categories.entrySet()) { categoriesList.add(category.getKey()); } return categoriesList.toArray(new String[0]); } /** * @return TreeModel root. */ public DefaultMutableTreeNode getRoot() { return root; } /** * @return DefaultTreeModel Whole tree model. */ public DefaultTreeModel getTreeModel() { treeModel.reload(); return treeModel; } /** * @return the categories */ public Map<String, Category> getCategoriesMap() { return categories; } } 

controller/Controller.java

package feeder.controller; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.util.NoSuchElementException; import javax.swing.JTree; import javax.swing.tree.DefaultMutableTreeNode; import javax.swing.tree.DefaultTreeModel; import com.rometools.rome.io.FeedException; import feeder.model.Channel; import feeder.model.News; import feeder.model.Model; import feeder.views.View; public class Controller { private Model model; public Controller() { model = loadModel(); Runtime.getRuntime().addShutdownHook(new Thread(new Runnable() { public void run() { saveModel(); } })); } private void saveModel() { try { FileOutputStream file = new FileOutputStream("data.dat"); ObjectOutputStream data = new ObjectOutputStream(file); data.writeObject(model); data.close(); file.close(); } catch (IOException e) { View.alertMessage("Wystąpił błąd w zapisie listy kanałów! Upewnij się, że masz prawa do zapisu na dysku."); } } private Model loadModel() { Model model = null; if(new File("data.dat").exists()) { FileInputStream file; try { file = new FileInputStream("data.dat"); model = (Model) new ObjectInputStream(file).readObject(); file.close(); } catch (ClassNotFoundException | IOException e) { View.alertMessage(e.getMessage()); } } else { model = new Model(); try { model.addNewCategory("Kategoria"); model.addNewChannel("BBC", "http://feeds.bbci.co.uk/news/rss.xml", "Kategoria"); } catch (IllegalArgumentException | FeedException | IOException e) { View.alertMessage(e.getMessage()); } } return model; } public void addNewFeedEvent(String name, String url, String category) throws IllegalArgumentException, FeedException, IOException { if(model.getCategoriesMap().get(category).getChannelsMap().get(name) != null) { model.addNewChannel(name, url, category); } else { View.alertMessage("Taki kanał w tej kategorii już istnieje!"); } } public void addNewCategoryEvent(String name) { if(model.getCategoriesMap().get(name) == null) { model.addNewCategory(name); } else { View.alertMessage("Kategoria już istnieje!"); } } public String[] getCategories() { return model.getCategoriesList(); } // maybe should've used something like Gagawa // but I think it's not worth the hassle public String convertNewsToHTML(Channel channel) { String html = "<html><h2 style='font-family: Tahoma'>" + channel.getName() + "</h2>"; try { for(News news : channel.getChannelContent()) { html += "<div style='font-family: Tahoma; font-size:13px; margin-bottom: 10px'>"; html += "<a style='font-weight: bold;' href='" + news.getLink() + "'>" + news.getTitle() + "</a>"; html += " (" + news.getDate() + ")<br>"; if(channel.isAggregated() == true) { html += "Z kanału: " + news.getChannel() + "<br>"; } html += news.getDescription() + "</div><hr>"; } } catch (IllegalArgumentException | FeedException | IOException e) { View.alertMessage("Wystąpił problem przy wczytywaniu nagłówków."); } return html; } public DefaultTreeModel getTreeModel() { return model.getTreeModel(); } public String elementInTreeFocusedEvent(JTree tree) throws FeedException, NoSuchElementException, IllegalArgumentException, IOException { DefaultMutableTreeNode node = (DefaultMutableTreeNode) tree.getLastSelectedPathComponent(); if (node == null) return null; Object nodeInfo = node.getUserObject(); DefaultMutableTreeNode category = (DefaultMutableTreeNode) node.getParent(); String content; if(category.equals(model.getRoot())) { // parameter is nodeInfo, NOT category, category contains Root content = convertNewsToHTML(model.getCategoryFeed(nodeInfo.toString())); } else { content = convertNewsToHTML(model.getChannelFeed(category.toString(), nodeInfo.toString())); } return content; } } 

view/View.java

package feeder.views; import java.awt.Component; import java.awt.Desktop; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; import java.io.IOException; import java.net.URISyntaxException; import java.util.NoSuchElementException; import javax.swing.GroupLayout; import javax.swing.GroupLayout.Alignment; import javax.swing.Icon; import javax.swing.ImageIcon; import javax.swing.JButton; import javax.swing.JEditorPane; import javax.swing.JFrame; import javax.swing.JOptionPane; import javax.swing.JPanel; import javax.swing.JScrollPane; import javax.swing.JTree; import javax.swing.LayoutStyle.ComponentPlacement; import javax.swing.UIManager; import javax.swing.UnsupportedLookAndFeelException; import javax.swing.border.EmptyBorder; import javax.swing.event.HyperlinkEvent; import javax.swing.event.HyperlinkListener; import javax.swing.event.TreeSelectionEvent; import javax.swing.event.TreeSelectionListener; import javax.swing.tree.DefaultMutableTreeNode; import javax.swing.tree.DefaultTreeCellRenderer; import javax.swing.tree.TreeSelectionModel; import com.rometools.rome.io.FeedException; import feeder.controller.Controller; public class View extends JFrame { private static final long serialVersionUID = 1L; private JButton btnAddFeed; private JButton btnAddCat; private JPanel ctpMain; private JTree tree; private JEditorPane feedPresenter; private Controller delegate; public View(Controller delegate) { this.delegate = delegate; try { UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName()); } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | UnsupportedLookAndFeelException e) { e.printStackTrace(); } initFrame(); eventListeners(); } private void initFrame() { //JFrame setTitle("Feeder v0.4"); setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE); setBounds(100, 100, 1024, 600); //JPanel - background ctpMain = new JPanel(); ctpMain.setBorder(new EmptyBorder(5, 5, 5, 5)); setContentPane(ctpMain); //Scrollable panels for content and tree JScrollPane scrPaneTree = new JScrollPane(); JScrollPane scrPaneContent = new JScrollPane(); //Buttons btnAddCat = new JButton("Dodaj kategori\u0119"); btnAddFeed = new JButton("Dodaj nowy kana\u0142"); //setting GroupLayout GroupLayout glCtpMain = new GroupLayout(ctpMain); glCtpMain.setHorizontalGroup( glCtpMain.createParallelGroup(Alignment.LEADING) .addGroup(glCtpMain.createSequentialGroup() .addGroup(glCtpMain.createParallelGroup(Alignment.LEADING, false) .addComponent(btnAddFeed, GroupLayout.DEFAULT_SIZE, GroupLayout.DEFAULT_SIZE, Short.MAX_VALUE) .addComponent(btnAddCat, GroupLayout.DEFAULT_SIZE, GroupLayout.DEFAULT_SIZE, Short.MAX_VALUE) .addComponent(scrPaneTree, GroupLayout.PREFERRED_SIZE, 161, GroupLayout.PREFERRED_SIZE)) .addPreferredGap(ComponentPlacement.RELATED) .addComponent(scrPaneContent, GroupLayout.DEFAULT_SIZE, 870, Short.MAX_VALUE)) ); glCtpMain.setVerticalGroup( glCtpMain.createParallelGroup(Alignment.LEADING) .addComponent(scrPaneContent, GroupLayout.DEFAULT_SIZE, 552, Short.MAX_VALUE) .addGroup(Alignment.TRAILING, glCtpMain.createSequentialGroup() .addComponent(scrPaneTree, GroupLayout.DEFAULT_SIZE, 483, Short.MAX_VALUE) .addPreferredGap(ComponentPlacement.RELATED) .addComponent(btnAddCat) .addPreferredGap(ComponentPlacement.RELATED) .addComponent(btnAddFeed) .addContainerGap()) ); feedPresenter = new JEditorPane(); feedPresenter.setEditable(false); feedPresenter.setEditorKit(JEditorPane.createEditorKitForContentType("text/html")); feedPresenter.setCaretPosition(0); tree = new JTree(); tree.setModel(delegate.getTreeModel()); tree.setShowsRootHandles(true); tree.setRootVisible(false); tree.getSelectionModel().setSelectionMode(TreeSelectionModel.SINGLE_TREE_SELECTION); tree.setCellRenderer(new DefaultTreeCellRenderer() { private static final long serialVersionUID = 1L; private Icon folderIcon = new ImageIcon("img/folder.png"); private Icon rssIcon = new ImageIcon("img/feed.png"); @Override public Component getTreeCellRendererComponent(JTree tree, Object value, boolean selected, boolean expanded, boolean isLeaf, int row, boolean focused) { Component component = super.getTreeCellRendererComponent(tree, value, selected, expanded, isLeaf, row, focused); DefaultMutableTreeNode node = (DefaultMutableTreeNode) value; if (tree.getModel().getRoot() == node.getParent()) setIcon(folderIcon); else setIcon(rssIcon); return component; } }); scrPaneContent.setViewportView(feedPresenter); scrPaneTree.setViewportView(tree); ctpMain.setLayout(glCtpMain); setVisible(true); } public static void alertMessage(String text) { JOptionPane.showMessageDialog(null, text); } public void eventListeners() { btnAddCat.addActionListener(new ActionListener() { @Override public void actionPerformed(ActionEvent argument) { NewCatDialog dialog = new NewCatDialog(delegate); dialog.setVisible(true); } }); btnAddFeed.addActionListener(new ActionListener() { @Override public void actionPerformed(ActionEvent argument) { NewFeedDialog dialog = new NewFeedDialog(delegate); dialog.setVisible(true); } }); feedPresenter.addHyperlinkListener(new HyperlinkListener() { @Override public void hyperlinkUpdate(HyperlinkEvent event) { if (event.getEventType() == HyperlinkEvent.EventType.ACTIVATED) { if (Desktop.isDesktopSupported()) { try { Desktop.getDesktop().browse(event.getURL().toURI()); } catch (IOException | URISyntaxException exception) { alertMessage(exception.getMessage()); } } } } }); tree.addTreeSelectionListener(new TreeSelectionListener() { @Override public void valueChanged(TreeSelectionEvent event) { try { String content = delegate.elementInTreeFocusedEvent(tree); feedPresenter.setText(content); feedPresenter.setCaretPosition(0); } catch (NoSuchElementException | IllegalArgumentException | FeedException | IOException exception) { alertMessage(exception.getMessage()); } } }); } } 

view/NewFeedDialog.java

package feeder.views; import java.awt.BorderLayout; import java.awt.FlowLayout; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; import java.io.IOException; import javax.swing.DefaultComboBoxModel; import javax.swing.GroupLayout; import javax.swing.GroupLayout.Alignment; import javax.swing.JButton; import javax.swing.JComboBox; import javax.swing.JDialog; import javax.swing.JLabel; import javax.swing.JPanel; import javax.swing.JTextField; import javax.swing.LayoutStyle.ComponentPlacement; import javax.swing.border.EmptyBorder; import com.rometools.rome.io.FeedException; import feeder.controller.Controller; public class NewFeedDialog extends JDialog { private static final long serialVersionUID = 1L; private final JPanel contentPanel = new JPanel(); private JLabel urlLabel; private JTextField urlField; private JTextField nameField; private JButton okButton; private JButton cancelButton; private JComboBox<String> categoryCmb; private Controller delegate; public NewFeedDialog(Controller delegate) { this.delegate = delegate; setTitle("Dodaj kanał RSS"); setResizable(false); setModal(true); setBounds(100, 100, 308, 180); setLocationRelativeTo(null); getContentPane().setLayout(new BorderLayout()); contentPanel.setBorder(new EmptyBorder(5, 5, 5, 5)); getContentPane().add(contentPanel, BorderLayout.CENTER); urlLabel = new JLabel("URL kanału"); JLabel nameLabel = new JLabel("Nazwa kanału"); JLabel categoryLabel = new JLabel("Kategoria"); urlField = new JTextField(); urlField.setColumns(10); nameField = new JTextField(); nameField.setColumns(10); categoryCmb = new JComboBox<String>(); categoryCmb.setModel(new DefaultComboBoxModel<String>(delegate.getCategories())); GroupLayout glContentPanel = new GroupLayout(contentPanel); glContentPanel.setHorizontalGroup( glContentPanel.createParallelGroup(Alignment.LEADING) .addGroup(glContentPanel.createSequentialGroup() .addContainerGap() .addGroup(glContentPanel.createParallelGroup(Alignment.LEADING) .addGroup(glContentPanel.createSequentialGroup() .addComponent(urlLabel) .addGap(18) .addComponent(urlField, GroupLayout.DEFAULT_SIZE, 200, Short.MAX_VALUE)) .addGroup(glContentPanel.createSequentialGroup() .addGroup(glContentPanel.createParallelGroup(Alignment.LEADING) .addComponent(nameLabel) .addComponent(categoryLabel)) .addPreferredGap(ComponentPlacement.RELATED) .addGroup(glContentPanel.createParallelGroup(Alignment.LEADING) .addComponent(categoryCmb, 0, 201, Short.MAX_VALUE) .addComponent(nameField, GroupLayout.DEFAULT_SIZE, 201, Short.MAX_VALUE)))) .addContainerGap()) ); glContentPanel.setVerticalGroup( glContentPanel.createParallelGroup(Alignment.LEADING) .addGroup(glContentPanel.createSequentialGroup() .addContainerGap() .addGroup(glContentPanel.createParallelGroup(Alignment.BASELINE) .addComponent(urlLabel) .addComponent(urlField, GroupLayout.PREFERRED_SIZE, GroupLayout.DEFAULT_SIZE, GroupLayout.PREFERRED_SIZE)) .addPreferredGap(ComponentPlacement.UNRELATED) .addGroup(glContentPanel.createParallelGroup(Alignment.BASELINE) .addComponent(nameLabel) .addComponent(nameField, GroupLayout.PREFERRED_SIZE, GroupLayout.DEFAULT_SIZE, GroupLayout.PREFERRED_SIZE)) .addPreferredGap(ComponentPlacement.UNRELATED) .addGroup(glContentPanel.createParallelGroup(Alignment.BASELINE) .addComponent(categoryLabel) .addComponent(categoryCmb, GroupLayout.PREFERRED_SIZE, GroupLayout.DEFAULT_SIZE, GroupLayout.PREFERRED_SIZE)) .addContainerGap(GroupLayout.DEFAULT_SIZE, Short.MAX_VALUE)) ); contentPanel.setLayout(glContentPanel); JPanel buttonPane = new JPanel(); buttonPane.setLayout(new FlowLayout(FlowLayout.RIGHT)); getContentPane().add(buttonPane, BorderLayout.SOUTH); okButton = new JButton("OK"); buttonPane.add(okButton); getRootPane().setDefaultButton(okButton); cancelButton = new JButton("Anuluj"); buttonPane.add(cancelButton); eventListeners(); } public void eventListeners() { cancelButton.addActionListener(new ActionListener() { @Override public void actionPerformed(ActionEvent argument) { dispose(); } }); okButton.addActionListener(new ActionListener() { @Override public void actionPerformed(ActionEvent argument) { try { delegate.addNewFeedEvent(nameField.getText(), urlField.getText(), categoryCmb.getSelectedItem().toString()); dispose(); } catch(IllegalArgumentException | FeedException | IOException exception) { View.alertMessage(exception.getMessage()); } } }); } } 

view/NewCatDialog.java

package feeder.views; import java.awt.BorderLayout; import java.awt.FlowLayout; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; import javax.swing.GroupLayout; import javax.swing.GroupLayout.Alignment; import javax.swing.JButton; import javax.swing.JDialog; import javax.swing.JLabel; import javax.swing.JPanel; import javax.swing.JTextField; import javax.swing.border.EmptyBorder; import feeder.controller.Controller; public class NewCatDialog extends JDialog { private static final long serialVersionUID = 1L; private final JPanel contentPanel = new JPanel(); private JButton okButton; private JButton cancelButton; private JTextField catNameField; private Controller delegate; public NewCatDialog(Controller delegate) { this.delegate = delegate; setTitle("Dodaj kategorię"); setResizable(false); setModal(true); setBounds(100, 100, 308, 118); setLocationRelativeTo(null); getContentPane().setLayout(new BorderLayout()); contentPanel.setBorder(new EmptyBorder(5, 5, 5, 5)); getContentPane().add(contentPanel, BorderLayout.CENTER); JLabel catNameLabel = new JLabel("Nazwa kategorii"); catNameField = new JTextField(); catNameField.setColumns(10); GroupLayout glContentPanel = new GroupLayout(contentPanel); glContentPanel.setHorizontalGroup( glContentPanel.createParallelGroup(Alignment.LEADING) .addGroup(glContentPanel.createSequentialGroup() .addGap(16) .addComponent(catNameLabel) .addGap(18) .addComponent(catNameField, GroupLayout.PREFERRED_SIZE, 150, GroupLayout.PREFERRED_SIZE) .addContainerGap(22, Short.MAX_VALUE)) ); glContentPanel.setVerticalGroup( glContentPanel.createParallelGroup(Alignment.LEADING) .addGroup(glContentPanel.createSequentialGroup() .addContainerGap() .addGroup(glContentPanel.createParallelGroup(Alignment.BASELINE) .addComponent(catNameLabel) .addComponent(catNameField, GroupLayout.PREFERRED_SIZE, 21, GroupLayout.PREFERRED_SIZE)) .addContainerGap(21, Short.MAX_VALUE)) ); contentPanel.setLayout(glContentPanel); JPanel buttonPane = new JPanel(); buttonPane.setLayout(new FlowLayout(FlowLayout.RIGHT)); getContentPane().add(buttonPane, BorderLayout.SOUTH); okButton = new JButton("OK"); buttonPane.add(okButton); getRootPane().setDefaultButton(okButton); cancelButton = new JButton("Anuluj"); buttonPane.add(cancelButton); eventListeners(); } public void eventListeners() { cancelButton.addActionListener(new ActionListener() { public void actionPerformed(ActionEvent argument) { dispose(); } }); okButton.addActionListener(new ActionListener() { @Override public void actionPerformed(ActionEvent argument) { try { dispose(); delegate.addNewCategoryEvent(catNameField.getText()); } catch(IllegalArgumentException exception) { View.alertMessage(exception.getMessage()); } } }); } } 
\$\endgroup\$
1
  • \$\begingroup\$Welcome to Code Review. Yes, you do need to include all of the code to be reviewed in the question, but you may also post a GitHub link to provide additional background, if the code to be reviewed is just an excerpt from your project.\$\endgroup\$CommentedJun 10, 2016 at 23:56

1 Answer 1

2
\$\begingroup\$

I think you have done a great job with your code so far. I think the best way for me to try to respond to you is for me to try to comment on each class one at a time.

First though I would like to suggest that Wikipedia has a pretty good description of the MVC pattern. That description is:

  1. A model stores data that is retrieved according to commands from the controller and displayed in the view.
  2. A view generates new output to the user based on changes in the model.
  3. A controller can send commands to the model to update the model's state (e.g. editing a document). It can also send commands to its associated view to change the view's presentation of the model (e.g. by scrolling through a document).

I will use this description as the basis for my comments on your classes.

You said:

Right now I use the Controller as a "bridge" between Model and View

Based on item #3 above I'd say that's the correct approach. The controller is supposed to act as the "bridge" between the other two pieces by sending commands back and forth between both the model and view.

Feeder.java

Considering just this class and none of the others, my comments are as follows: You have written new View(new Controller()); which seems to say that the View knows about the Controller but not vice versa. This does not seem to conform to the MVC pattern as described above in items 1-3. If the Controller is to act as the bridge between model and view then I would expect the view knows about the Controller (i.e. has a reference to it) and likewise the View should have a reference to the Controller in order for the two to communicate.

Controller.java

Looking at this class I see it does have a field of type Model, which I would expect to be part of the data model based on the name. However, it appears to be part of the view. Please see my comments on Model.java further down.

The methods saveModel and loadModel are good in that I think they're in the right place. The Controller should be responsible for reading data and updating the data model as well as saving the data. (See item #1)

The method convertNewsToHTML might be better placed in the view since HTML is essentially a way to view the data. I also think your concern about elementInTreeFocusedEvent is justified, and it would probably be more appropriately placed in the view - especially because it deals with JTree which is a class from swing meant for displaying data.

Model.java

I see this class is part of the model package, but it uses some classes that are part of the view. For example:

private DefaultMutableTreeNode root; private DefaultTreeModel treeModel; 

The classes DefaultMutableTreeNode and DefaultTreeModel are part of swing, which means they are used by swing to display data. They should be considered part of the view based on items 1-3 above.

I also notice that this class is calling methods like the following:

treeModel.insertNodeInto treeModel.reload(); 

Again, based on items 1-3 these are probably the kinds of calls that should be made from controller logic since the controller is, as you said, the "bridge" between model and view.

Category.java, Channel.java, News.java

These classes look good. They seem to be standard data model classes.

NewCatDialog.java, NewFeedDialog.java

These classes look good also. They seem to be handling only what they should - view related data and logic.

View.java

This class looks like mostly view code, but it calls methods on the delegate (whose type is Controller) like the following:

delegate.getTreeModel() 

I already mentioned before that based on items 1-3 the controller should have references to the model and view and pass messages between them. With this in mind I don't think the View should, according to MVC, have to ask the Controller for the TreeModel since the TreeModel should already be part of the view somehow.

Final Thoughts

MVC is widely considered to be best practice when designing software. However, that doesn't mean that it constitutes a set of exacting rules that must always be adhered to. Trying to strictly adhere to the separation of model, view, controller can be tricky and may take several revisions. I find it's even trickier when starting out with swing because it's easy to confuse the "model" classes that swing uses as part of the data model. Remember swing is the view, anything to do with swing should be view related. I hope all of this helps you, best of luck with your code!

\$\endgroup\$

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.