CSV File Reader Project

Question

I'm trying to focus on learning so I can get an entry level position and the best way for me to learn right now is to have someone review and provide areas of improvement. This project isn't entirely complete but almost. I am going to include the code for some of the classes but not all of them. Just a few button actions left to finish. All criticism is welcomed.

public class CreateAndShowUI {
 public static void createUI() {
 Model model = new Model();
 MainPanel mainPanel = new MainPanel(model, new CSVFileController(model));
 JFrame frame = new JFrame();
 frame.getContentPane().add(mainPanel.getMainPanel());
 frame.setUndecorated(true);
 frame.setPreferredSize(new Dimension(1100, 550));
 frame.addMouseListener(new ApplicationMouseAdapters.FrameMouseAdapter());
 frame.addMouseMotionListener(new ApplicationMouseAdapters.FrameMouseMotionListener(
 frame));
 frame.pack();
 frame.setLocationRelativeTo(null);
 frame.setVisible(true);
 }
 public static final Font getHeaderFont() {
 return new Font("Consolas", Font.BOLD, 16);
 }
 public static final Font getFont() {
 return new Font("Consolas", Font.BOLD, 14);
 }
 public static final Font getTableFont() {
 return new Font("Consolas", Font.PLAIN, 16);
 }
 public static final Color getButtonColor() {
 return new Color(230, 230, 230);
 }
 public static void main(String[] args) {
 createUI();
 }
}

The MainPanel Class which basically creates the main JPanel for the JFrame. This JPanel has two child panels and one menu bar.

public class MainPanel {
 private Model model;
 private CSVFileController controller;
 private JPanel mainPanel;
 private Dialog dialog;
 private MenuBar menuBar;
 private ButtonPanel buttonPanel;
 private JScrollPane buttonScrollPane, listScrollPane;
 private JTable table;
 private JLabel search;
 private JTextField searchTF;
 private JButton xButton;
 public MainPanel(Model model, CSVFileController controller) {
 this.model = model;
 this.controller = controller;
 createMainPanel();
 }
 private void createMainPanel() {
 mainPanel = new JPanel(new MigLayout("", "", "[]13[]"));
 dialog = new Dialog();
 table = new JTable(new ProductTableModel());
 setTableColumnWidth(table.getModel());
 table.getTableHeader()
 .setPreferredSize(
 new Dimension(table.getColumnModel()
 .getTotalColumnWidth(), 48));
 table.getTableHeader().setResizingAllowed(false);
 table.getTableHeader().setReorderingAllowed(false);
 table.getTableHeader().setFont(CreateAndShowUI.getFont());
 table.setFont(CreateAndShowUI.getTableFont());
 table.addMouseListener(new ApplicationMouseAdapters.TableMouseAdapter(
 dialog, table.getModel(), controller));
 setTableEditorFont(table);
 menuBar = new MenuBar();
 mainPanel.add(menuBar.getMenuBar());
 search = new JLabel("Search");
 mainPanel.add(search, "cell 0 0, gap 10px");
 searchTF = new JTextField("", 30);
 mainPanel.add(searchTF, "cell 0 0");
 xButton = new JButton(new CloseAction("X"));
 xButton.setFocusable(false);
 xButton.setFont(CreateAndShowUI.getFont());
 xButton.setBackground(new Color(158, 7, 7));
 xButton.setForeground(Color.WHITE);
 mainPanel.add(xButton, "cell 0 0, gap 283px, wrap");
 buttonPanel = new ButtonPanel();
 buttonScrollPane = new JScrollPane(buttonPanel.getButtonPanel(),
 JScrollPane.VERTICAL_SCROLLBAR_AS_NEEDED,
 JScrollPane.HORIZONTAL_SCROLLBAR_NEVER);
 buttonScrollPane.setViewportView(buttonPanel.getButtonPanel());
 buttonScrollPane.setPreferredSize(new Dimension(250, 990));
 mainPanel.add(buttonScrollPane);
 listScrollPane = new JScrollPane(table,
 JScrollPane.VERTICAL_SCROLLBAR_AS_NEEDED,
 JScrollPane.HORIZONTAL_SCROLLBAR_NEVER);
 listScrollPane.setPreferredSize(new Dimension(850, 990));
 mainPanel.add(listScrollPane, "cell 0 1");
 }
 public JPanel getMainPanel() {
 return mainPanel;
 }
 public JPanel getButtonPanel() {
 return buttonPanel.getButtonPanel();
 }
 public JTable getTable() {
 return table;
 }
 public JMenuBar getMenuBar() {
 return menuBar.getMenuBar();
 }
 public final void setTableEditorFont(JTable table) {
 DefaultCellEditor editor = (DefaultCellEditor) table
 .getDefaultEditor(Object.class);
 Component component = editor.getComponent();
 component.setFont(table.getFont());
 editor.setClickCountToStart(1);
 }
 public final void setTableColumnWidth(TableModel tableModel) {
 final TableColumnModel columnModel = table.getColumnModel();
 if (tableModel.getColumnCount() == 6) {
 columnModel.getColumn(0).setPreferredWidth(200);
 columnModel.getColumn(1).setPreferredWidth(300);
 columnModel.getColumn(2).setPreferredWidth(80);
 columnModel.getColumn(3).setPreferredWidth(75);
 columnModel.getColumn(4).setPreferredWidth(80);
 columnModel.getColumn(5).setPreferredWidth(75);
 }
 }
 public class ButtonPanel {
 private JPanel panel;
 public ButtonPanel() {
 panel = new JPanel(new MigLayout());
 addButtons();
 }
 private void addButtons() {
 List<Supplier> supplierList = model.getSuppliers();
 for (Supplier supplier : supplierList) {
 JButton button = new JButton(
 new SupplierButtonActions.ShowSupplierProductsAction(
 supplier.getName(), getTable(), model));
 button.setFocusable(false);
 button.setFont(CreateAndShowUI.getFont());
 button.setBackground(CreateAndShowUI.getButtonColor());
 button.setPreferredSize(new Dimension(230, 30));
 button.setHorizontalAlignment(SwingConstants.CENTER);
 panel.add(button, "wrap");
 }
 }
 public JPanel getButtonPanel() {
 return panel;
 }
 }
 public class MenuBar {
 private JMenuBar menuBar;
 private JMenu application, suppliers, orders;
 private JMenuItem viewOrders, newSupplier, addProduct, close;
 public MenuBar() {
 createMenuBar();
 }
 private void createMenuBar() {
 menuBar = new JMenuBar();
 close = new JMenuItem(new CloseAction("Close"));
 close.setPreferredSize(new Dimension(125, 25));
 close.setFont(CreateAndShowUI.getHeaderFont());
 application = new JMenu("Application");
 application.setFont(CreateAndShowUI.getHeaderFont());
 application.setPreferredSize(new Dimension(125, 70));
 application.add(close);
 menuBar.add(application);
 newSupplier = new JMenuItem(
 new DialogActions.AddSupplierPanelAction(dialog, "New"));
 newSupplier.setPreferredSize(new Dimension(125, 25));
 newSupplier.setFont(CreateAndShowUI.getHeaderFont());
 addProduct = new JMenuItem(new DialogActions.AddProductPanelAction(
 dialog, model, "Add Products"));
 addProduct.setPreferredSize(new Dimension(125, 25));
 addProduct.setFont(CreateAndShowUI.getHeaderFont());
 suppliers = new JMenu("Suppliers");
 suppliers.setFont(CreateAndShowUI.getHeaderFont());
 suppliers.setPreferredSize(new Dimension(125, 70));
 suppliers.add(newSupplier);
 suppliers.add(addProduct);
 menuBar.add(suppliers);
 viewOrders = new JMenuItem("View");
 viewOrders.setFont(CreateAndShowUI.getHeaderFont());
 viewOrders.setPreferredSize(new Dimension(125, 25));
 orders = new JMenu("Orders");
 orders.setFont(CreateAndShowUI.getHeaderFont());
 orders.setPreferredSize(new Dimension(125, 70));
 orders.add(viewOrders);
 menuBar.add(orders);
 }
 public JMenuBar getMenuBar() {
 return menuBar;
 }
 }
}

The KeywordPanel class which is one of the panels for JDialog. This panel is added dynamically to a dialog based on a specific action taken by the user.

 public class KeywordPanel {
 private CSVFileController controller;
 private Product product;
 private Dialog dialog;
 private JPanel keywordPanel;
 private JTextField[] textFields;
 private JLabel searchLbl;
 private JButton close, remove;
 public KeywordPanel(CSVFileController controller, Dialog dialog,
 Product product) {
 this.controller = controller;
 this.product = product;
 this.dialog = dialog;
 createKeywordPanel();
 }
 public void createKeywordPanel() {
 keywordPanel = new JPanel(new MigLayout());
 searchLbl = new JLabel("KeyWords");
 searchLbl.setFont(CreateAndShowUI.getFont());
 keywordPanel.add(searchLbl, "wrap");
 textFields = new JTextField[] { new JTextField(""), new JTextField(""),
 new JTextField(""), new JTextField(""), new JTextField("") };
 for (JTextField textField : textFields) {
 textField.setFont(CreateAndShowUI.getFont());
 textField.setPreferredSize(new Dimension(242, 30));
 keywordPanel.add(textField, "wrap");
 }
 remove = new JButton(new KeywordPanelController.RemoveAction("Remove",
 controller, product));
 remove.setBackground(CreateAndShowUI.getButtonColor());
 remove.setFont(CreateAndShowUI.getFont());
 keywordPanel.add(remove);
 close = new JButton(new KeywordPanelController.SaveAndCloseAction(
 "Save & Close", controller, dialog, product, textFields));
 close.setBackground(CreateAndShowUI.getButtonColor());
 close.setFont(CreateAndShowUI.getFont());
 keywordPanel.add(close, "cell 0 6, gapx 30px");
}
public void setCategoryTextFields(int index, String category) {
 textFields[index]
 .addMouseListener(new KeywordPanelController.TextFieldMouseAdapter(
 textFields[index]));
 textFields[index].setEditable(false);
 textFields[index].setBackground(Color.WHITE);
 textFields[index].setText(category);
 }
 public void setLocation(int x, int y) {
 keywordPanel.setLocation(x, y);
 }
 public JPanel getKeywordPanel() {
 return keywordPanel;
 }
}

Main model for the application:

public class Model {
 private Map<Supplier, List<Product>> supplierProductList;
 private List<Product> productList;
 public Model() {
 this.supplierProductList = new TreeMap<Supplier, List<Product>>();
 }
 public void addSupplier(Supplier supplier) {
 if (!supplierProductList.containsKey(supplier)) {
 this.supplierProductList.put(supplier, new ArrayList<Product>());
 }
 }
 public ArrayList<Supplier> getSuppliers() {
 return new ArrayList<Supplier>(supplierProductList.keySet());
 }
 public void addProduct(String name, Product product) {
 for (Supplier supplier : supplierProductList.keySet()) {
 if (supplier.getName().equals(name)
 && !supplierProductList.get(name).contains(product)) {
 this.productList = supplierProductList.get(name);
 this.productList.add(product);
 this.supplierProductList.put(supplier, productList);
 }
 }
 }
 public void addProductList(Supplier supplier, List<Product> productList) {
 this.supplierProductList.put(supplier, productList);
 }
 public ArrayList<Product> getProducts(String name) {
 for (Supplier supplier : getSuppliers()) {
 if (supplier.getName().equals(name))
 return new ArrayList<Product>(supplierProductList.get(supplier));
 }
 return null;
 }
}

This is the CSVFileController class. This is used to update the model and the files based on user interaction with the view.

public class CSVFileController {
 private Model model;
 private BufferedReader reader;
 private BufferedWriter writer;
 private String line;
 public CSVFileController(Model model) {
 this.model = model;
 this.reader = null;
 this.writer = null;
 this.line = "";
 updateModel();
 }
 public void updateModel() {
 List<Supplier> suppliers = new ArrayList<Supplier>();
 try {
 reader = new BufferedReader(new FileReader("Suppliers.csv"));
 while ((line = reader.readLine()) != null) {
 String[] dataArray = line.split(",");
 suppliers.add(new Supplier(dataArray[0], Integer
 .parseInt(dataArray[1])));
 }
 for (Supplier supplier : suppliers) {
 List<Product> productList = new ArrayList<Product>();
 reader = new BufferedReader(new FileReader(supplier.getName()
 + ".csv"));
 while ((line = reader.readLine()) != null) {
 String[] dataArray = line.split(",");
 if (dataArray.length == 6) {
 productList.add(new Product(supplier, dataArray[0],
 dataArray[1], Integer.parseInt(dataArray[2]),
 Integer.parseInt(dataArray[3]), Double
 .parseDouble(dataArray[4]), Integer
 .parseInt(dataArray[5])));
 } else {
 List<String> categories = new ArrayList<String>();
 for (int x = 6; x < dataArray.length; x++) {
 categories.add(dataArray[x]);
 }
 productList.add(new Product(supplier, dataArray[0],
 dataArray[1], Integer.parseInt(dataArray[2]),
 Integer.parseInt(dataArray[3]), Double
 .parseDouble(dataArray[4]), Integer
 .parseInt(dataArray[5]), categories));
 }
 }
 model.addProductList(supplier, productList);
 }
 } catch (FileNotFoundException e) {
 e.printStackTrace();
 } catch (IOException e) {
 e.printStackTrace();
 } finally {
 closeReader(reader);
 }
 }
 public void adjustProductCategories(Product product) {
 List<Product> supplierProductList = model.getProducts(product
 .getSupplier().getName());
 try {
 writer = new BufferedWriter(new FileWriter(product.getSupplier()
 .getName() + ".csv"));
 for (ListIterator<Product> iterator = supplierProductList
 .listIterator(); iterator.hasNext();) {
 Product prod = iterator.next();
 if (prod.getId() == product.getId()) {
 iterator.remove();
 iterator.add(product);
 model.addProductList(product.getSupplier(),
 supplierProductList);
 }
 writer.write(prod.toString());
 writer.newLine();
 }
 } catch (IOException ex) {
 ex.printStackTrace();
 } finally {
 closeWriter(writer);
 }
 }
 public void closeWriter(BufferedWriter bw) {
 try {
 if (bw != null) {
 bw.close();
 }
 } catch (IOException e) {
 e.printStackTrace();
 }
 }
 public void closeReader(BufferedReader br) {
 try {
 if (br != null) {
 br.close();
 }
 } catch (IOException e) {
 e.printStackTrace();
 }
 }
}

This is the controller for the KeywordPanel:

 public class KeywordPanelController {
 public static class RemoveAction extends TextAction {
 private CSVFileController controller;
 private Product product;
 public RemoveAction(String name, CSVFileController controller,
 Product product) {
 super(name);
 this.controller = controller;
 this.product = product;
 }
 @Override
 public void actionPerformed(ActionEvent e) {
 JTextComponent textField = (JTextField) getFocusedComponent();
 if (!textField.isEditable()) {
 product.getCategories().remove(textField.getText().trim());
 product.setCategories(product.getCategories());
 controller.adjustProductCategories(product);
 textField.setText("");
 textField.setEditable(true);
 }
 }
 }
 public static class TextFieldMouseAdapter extends MouseAdapter {
 private JTextField textField;
 public TextFieldMouseAdapter(JTextField textField) {
 this.textField = textField;
 }
 @Override
 public void mouseClicked(MouseEvent evt) {
 textField.setSelectionColor(new Color(142, 15, 6));
 textField.setSelectedTextColor(new Color(255, 255, 255));
 textField.setSelectionStart(0);
 textField.setSelectionEnd(textField.getText().trim().length());
 }
 }
 public static class SaveAndCloseAction extends AbstractAction {
 private JTextField[] textFields;
 private Product product;
 private CSVFileController controller;
 private Dialog dialog;
 public SaveAndCloseAction(String name, CSVFileController controller,
 Dialog dialog, Product product, JTextField[] textFields) {
 super(name);
 this.controller = controller;
 this.textFields = textFields;
 this.product = product;
 this.dialog = dialog;
 }
 @Override
 public void actionPerformed(ActionEvent e) {
 List<String> categories = new ArrayList<String>();
 for (JTextField textField : textFields) {
 if (new ValidateString().validate(textField.getText().trim())) {
 categories.add(textField.getText().trim());
 }
 }
 product.setCategories(categories);
 controller.adjustProductCategories(product);
 dialog.getDialog().dispose();
 }
 }
}

The Product class which is a POJO. I'm not entirely sure if this should be considered a model class or not.

public class Product {
 private Supplier supplier;
 private List<String> categoryList;
 private int minQty, qtyOnHand, orderQty;
 private double cost;
 private String productDescription, id;
 public Product(Supplier supplier, String id, String productDescription,
 int qtyOnHand, int minQty, double cost, int orderQty,
 List<String> categories) {
 this.supplier = supplier;
 this.qtyOnHand = qtyOnHand;
 this.orderQty = orderQty;
 this.id = id;
 this.minQty = minQty;
 this.cost = cost;
 this.productDescription = productDescription;
 this.categoryList = categories;
 }
 public Product(Supplier supplier, String id, String productDescription,
 int qtyOnHand, int minQty, double cost, int orderQty) {
 this.supplier = supplier;
 this.qtyOnHand = qtyOnHand;
 this.orderQty = orderQty;
 this.id = id;
 this.minQty = minQty;
 this.cost = cost;
 this.productDescription = productDescription;
 }
 public Product(String id, String productDescription, int qtyOnHand,
 int minQty, double cost, int orderQty) {
 this.qtyOnHand = qtyOnHand;
 this.orderQty = orderQty;
 this.id = id;
 this.minQty = minQty;
 this.cost = cost;
 this.productDescription = productDescription;
 }
 public int getQtyOnHand() {
 return qtyOnHand;
 }
 public void setQtyOnHand(int qtyOnHand) {
 this.qtyOnHand = qtyOnHand;
 }
 public int getOrderQty() {
 return orderQty;
 }
 public void setOrderQty(int orderQty) {
 this.orderQty = orderQty;
 }
 public String getId() {
 return id;
 }
 public void setId(String id) {
 this.id = id;
 }
 public double getCost() {
 return cost;
 }
 public void setCost(double cost) {
 this.cost = cost;
 }
 public String getProductDescription() {
 return productDescription;
 }
 public void setProductDescription(String productDescription) {
 this.productDescription = productDescription;
 }
 public int getMinQty() {
 return minQty;
 }
 public void setMinQty(int minQty) {
 this.minQty = minQty;
 }
 public List<String> getCategories() {
 return categoryList;
 }
 public void setCategories(List<String> categories) {
 this.categoryList = categories;
 }
 public Supplier getSupplier() {
 return supplier;
 }
 public void setSupplier(Supplier supplier) {
 this.supplier = supplier;
 }
 public boolean isCategorized() {
 if (categoryList == null) {
 return false;
 } else {
 return true;
 }
 }
 @Override
 public String toString() {
 StringBuilder builder = new StringBuilder();
 builder.append(id).append(",").append(productDescription).append(",")
 .append(qtyOnHand).append(",").append(minQty).append(",")
 .append(cost).append(",").append(orderQty);
 if (isCategorized()) {
 for (String category : categoryList) {
 builder.append(",").append(category);
 }
 }
 return builder.toString();
 }
}

Answer 1

Let me start with something good: the names of the variables are good. Even if there is no recognizable package organization: I have seen more bad code from entry-level programmers.

If ever there is any AWT/Swing job, I'd love to use NetBeans as UI-Designer, and you would love it. But hey, to code everything on your own is ok to learn, I think.

Let's get to the critiques:

1. The name of the class CreateAndShowUI tries to explain what the implementation does. This hurt particularly the encapsulation and a few principles of SOLID.

2. The fonts inside of CreateAndShowUI should be fields, if you place it into a interface you don't need to write public static final because they are immanent by interfacefields.

3. Have you ever thought about extending the MainPanel from JFrame? I think the separation of a instance of MainPanel and JFrame is unnecessary. If you do so, please rename MainPanel to MainFrame. Also, ButtonPanel can extend JPanel and MenuBar can extend JMenuBar.

4. Some of the fields in MainPanel can be final fields. You don't need a createMainPanel function.

   A short example:

   private final JPanel mainPanel = new JPanel(new MigLayout(...));
   private final JTable table = new JTable(new ProductTableModel());
   private final JScrollPane listScrollPane = new JScrollPane(table, ...);
   private final JButton xButton = new JButton(new CloseAction("X"));
   

5. In Model, the productList should be a variable in the method addProduct, not a field. supplierProductList should have a Set instead of a List as a value.

6. The CSVFileController is not thread-safe. If you doubleclick too fast on adjustProductCategories, the line writer = new BufferedWriter(...) creates two instances, but the field writer will hold only one instance. So closeWriter(writer); will executed twice to the writer you created at last and the writer of the first click will never be closed!

7. Product is not just a POJO; you created a perfect java-bean. One problem: No one expected to have called isCategorized if you call toString. This is an implementation detail you should hide. Inline categoryList==null in toString please, so everyone is allowed to throw NotImplementedException in isCategorized without throwing it in the toString method too. But keep the method isCategorized as it is.

   If any class (i.e. SpecialProduct) implements Product who has a special kind of isCategorized, then toString may return wrong values. This would be a bug inside SpecialProduct but it's a very bad leak in the design.

• Grim: Thank You for your response, I will be working on this first thing tomorrow.
• Grim: stackoverflow.com/questions/320588/… According to this, an interface with only fields is considered bad practice.
• Grim: stackoverflow.com/questions/22003802/… According to this, extending or not extending a container is based on preference
• Grim: @CharlesSexton I confirm to use constants in interfaces that are implemented is bad practice. But if we never implement the interface the arguments are reduced: 1. The duplicated JavaDoc is no argument anymore because there is only one documentation.2.The instanceof-smell is no argument because if noone implement the interface you can not call "instance-of" on the instance.3.The argument "if in a future release the class is modified" but no argument anymore because its not part of a official api for 3rd partys. The argument that its not the intend of an interface i say: Correct, but its SOLID
• Grim: @CharlesSexton The extending or not extending argument is based of the open-close-principle + the request to change the core-behave of the class at runtime. But as we dont need to change the core-behave of the iframe (except the constructor) its ok i think.