Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have created a program using a version of MVC architecture. The purpose of the code is to scrape the h1 titles of a list of webpages and to return the results to a JTable.

So far I have the program working fine. It's returning the results just as I want them to but it doesn't update the table until the very end. I want it to update the table as the results come in. I want to do this in a way that take best practice principals into account as I am just learning.

I presume to get this to update as I want it I will have to change my code around a bit. I'm not sure of the best way to update the GUI dynamically(threads, observers, something else?). I'm not even sure if the question "where in my MVC pattern should this code sit?" makes sense?

Anyways here is my View:

public class SearchView extends JFrame{
//Components
private JLabel selectElementLabel = new JLabel("Element Selector:");
private JTextField selectElement = new JTextField("h1");;
private JComboBox<String> selectLocale; 

private DefaultTableModel tableModel = new DefaultTableModel();
private JTable resultTable = new JTable(tableModel);

private JLabel statusLabel;
private JButton runButton = new JButton("Run");
private JButton clearButton = new JButton("Clear");

private SearchModel s_model;

//Constructor
public SearchView(SearchModel model) {
    //Set the Logic here(model)
    s_model = model;    

    //Initialise Components here(model)
    selectLocale = new JComboBox<>(s_model.getLocales());
    selectLocale.setSelectedIndex(13);

    //Layout Components
    JPanel userInputPanel = new JPanel();
    userInputPanel.setLayout(new BoxLayout(userInputPanel, BoxLayout.X_AXIS));
    userInputPanel.add(selectElementLabel);
    userInputPanel.add(selectElement);
    userInputPanel.add(selectLocale);

    tableModel.addColumn("Page");
    tableModel.addColumn("Data");
    resultTable.setFillsViewportHeight(true);

    JScrollPane resultScroller = new JScrollPane(resultTable);
    resultScroller.setVerticalScrollBarPolicy(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS);
    resultScroller.setHorizontalScrollBarPolicy(ScrollPaneConstants.HORIZONTAL_SCROLLBAR_AS_NEEDED);
    resultScroller.setAlignmentX(Component.LEFT_ALIGNMENT);

    JPanel controlButtons = new JPanel();
    controlButtons.setLayout(new FlowLayout(FlowLayout.RIGHT));
    controlButtons.add(statusLabel = new JLabel(s_model.getState()));
    controlButtons.add(clearButton);
    controlButtons.add(runButton);


    this.setTitle("Element Searcher");
    this.add(BorderLayout.NORTH, userInputPanel);
    this.add(BorderLayout.CENTER, resultScroller);
    this.add(BorderLayout.SOUTH, controlButtons);
    this.setExtendedState(Frame.MAXIMIZED_BOTH); 
    this.setMinimumSize(new Dimension(900, 600));
    this.setVisible(true);  
    this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);        
}

void reset(){
    tableModel.setRowCount(0);
}

String getSelectedElement(){
    return selectElement.getText();
}

String getSelectedLocale(){
    return selectLocale.getSelectedItem().toString();
}

void setResults(Object[] result){
    tableModel.addRow(result);
}

void addRunListener(ActionListener run){
    runButton.addActionListener(run);
}

void addClearListerner(ActionListener clear){
    clearButton.addActionListener(clear);
}

}

Controller:

public class SearchController {

private SearchModel s_model;
private SearchView s_view;

public SearchController(SearchModel model, SearchView view) {
    s_model = model;
    s_view = view;

    s_view.addRunListener(new RunListener());
    s_view.addClearListerner(new ClearListener());

}

class RunListener implements ActionListener{
    public void actionPerformed(ActionEvent e){
        String selectedLocale = null;
        try {
            selectedLocale = s_view.getSelectedLocale();
            s_model.setPageList(selectedLocale);
            for (String pageUrl : s_model.getPageList()){
                s_view.setResults(s_model.getResults(pageUrl));
            }
        } catch (Exception e1) {
            System.out.println(e1);
        }
    }
}

class ClearListener implements ActionListener{
    public void actionPerformed(ActionEvent e){
        s_model.reset();
        s_view.reset();
    }
}

}

and finally my model:

public class SearchModel {
//Constants
private static final String[] localeStrings = { "cs-cz", "da-dk", "de-at", "de-ch", "de-de", "el-gr", "en-ae", "en-au", "en-ca", "en-gb", "en-ie", "en-in", "en-nz", "en-us", "en-za", "es-cl", "es-co", "es-es", "es-mx", "fi-fi", "fr-be", "fr-ca", "fr-ch", "fr-fr", "hu-hu", "it-it", "ja-jp", "ko-kr", "nb-no", "nl-be", "nl-nl", "pl-pl", "pt-br", "pt-pt", "ru-ru", "sk-sk", "sv-se", "zh-hk", "zh-sg", "zh-tw" };
private static final String INITIAL_STATE = "idle";
private HashSet<String> pageList;
private Object[] scrapeResult;
private String locale = "en-us";

//Search State
private String searchState;

public SearchModel() {
    reset();
}

public void setPageList(String loc){
    locale = loc;
    ScrapeXML scraper = new ScrapeXML(locale);
    pageList = scraper.getUrls();
}

public void setResults(String page){
    ScrapeElements scraper = new ScrapeElements(page, locale);
    scrapeResult = scraper.getResults();
}

public void reset(){
    searchState = INITIAL_STATE;
}

public String[] getLocales(){
    return localeStrings;
}

public String getState(){
    return searchState;
}

public HashSet<String> getPageList(){
    return pageList;
}

public Object[] getResults(String page){
    setResults(page);
    return scrapeResult;
}

}
share|improve this question

1 Answer 1

Since your code is not runnable (the ScrapeXML and ScrapeElements classes are missing), I can't answer your main question about event processing.

However, in your view, you should use composition instead of inheritance. I've modified your view to make the JFrame a component.

import java.awt.BorderLayout;
import java.awt.Component;
import java.awt.Dimension;
import java.awt.FlowLayout;
import java.awt.Frame;
import java.awt.event.ActionListener;

import javax.swing.BoxLayout;
import javax.swing.JButton;
import javax.swing.JComboBox;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JScrollPane;
import javax.swing.JTable;
import javax.swing.JTextField;
import javax.swing.ScrollPaneConstants;
import javax.swing.table.DefaultTableModel;

public class SearchView {
    // Components
    private JFrame              frame               = new JFrame();

    private JLabel              selectElementLabel  = new JLabel(
                                                            "Element Selector:");
    private JTextField          selectElement       = new JTextField("h1");         ;
    private JComboBox           selectLocale;

    private DefaultTableModel   tableModel          = new DefaultTableModel();
    private JTable              resultTable         = new JTable(tableModel);

    private JLabel              statusLabel;
    private JButton             runButton           = new JButton("Run");
    private JButton             clearButton         = new JButton("Clear");

    private SearchModel         s_model;

    // Constructor
    public SearchView(SearchModel model) {
        // Set the Logic here(model)
        s_model = model;

        // Initialise Components here(model)
        selectLocale = new JComboBox(s_model.getLocales());
        selectLocale.setSelectedIndex(13);

        // Layout Components
        JPanel userInputPanel = new JPanel();
        userInputPanel
                .setLayout(new BoxLayout(userInputPanel, BoxLayout.X_AXIS));
        userInputPanel.add(selectElementLabel);
        userInputPanel.add(selectElement);
        userInputPanel.add(selectLocale);

        tableModel.addColumn("Page");
        tableModel.addColumn("Data");
        resultTable.setFillsViewportHeight(true);

        JScrollPane resultScroller = new JScrollPane(resultTable);
        resultScroller.setVerticalScrollBarPolicy(
            ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS);
        resultScroller.setHorizontalScrollBarPolicy(
            ScrollPaneConstants.HORIZONTAL_SCROLLBAR_AS_NEEDED);
        resultScroller.setAlignmentX(Component.LEFT_ALIGNMENT);

        JPanel controlButtons = new JPanel();
        controlButtons.setLayout(new FlowLayout(FlowLayout.RIGHT));
        controlButtons.add(statusLabel = new JLabel(s_model.getState()));
        controlButtons.add(clearButton);
        controlButtons.add(runButton);

        frame.setTitle("Element Searcher");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.add(BorderLayout.NORTH, userInputPanel);
        frame.add(BorderLayout.CENTER, resultScroller);
        frame.add(BorderLayout.SOUTH, controlButtons);
        frame.setExtendedState(Frame.MAXIMIZED_BOTH);
        frame.setMinimumSize(new Dimension(900, 600));
        frame.setVisible(true);
    }

    public JFrame getFrame() {
        return frame;
    }

    void reset() {
        tableModel.setRowCount(0);
    }

    String getSelectedElement() {
        return selectElement.getText();
    }

    String getSelectedLocale() {
        return selectLocale.getSelectedItem().toString();
    }

    void setResults(Object[] result) {
        tableModel.addRow(result);
    }

    void addRunListener(ActionListener run) {
        runButton.addActionListener(run);
    }

    void addClearListerner(ActionListener clear) {
        clearButton.addActionListener(clear);
    }

}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.