2
\$\begingroup\$

I have a MVC design for a GUI Java program. I was wondering if:

  1. Someone could either comment the code to help me out in understanding what I am doing better.

  2. Help me fix newbie mistakes to make this program function better.

This program asks the user for some info about a train ticket and trip and basically processes a card they enter and spits out a receipt if the card is valid. The requirement is that I use the MVC design pattern with the catch that there has to be 3 packages of model, view, and controller.

I can upload this to Google Drive if need be.

CONTROLLER PACKAGE

package edu.witc.TrainTicket.controller;

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.text.NumberFormat;

import javax.swing.JOptionPane;

import edu.witc.TrainTicket.model.*;
import edu.witc.TrainTicket.view.*;


public class TrainTicketController {

    private MainForm view = null;
    private Destination model = null;
    private CreditCard model1 = null;


    public TrainTicketController(MainForm mainform, Destination destination, CreditCard card){
        this.view = mainform;
        this.model = destination;
        this.model1 = card;


        //add listener
        SubmitButtonListener submit = new SubmitButtonListener();
        this.view.addSubmitListener(submit);

    }


class SubmitButtonListener implements ActionListener{

        public void actionPerformed(ActionEvent e) {

            //set variables
            boolean validCard = false;
            boolean validName = false;
            boolean validPhone = false;
            boolean validPaintSelect = false;
            String destinationSelected = "";
            int count = 0;

            Destination destination = new Destination(view.jrbChicago,view.jrbNewYork, view.jrbSeattle,view.jrbSanFransisco, view.jtfName, view.jtfPhone);

            CreditCard card = new CreditCard();     

            //validate fields
            ValidateFields validate = new ValidateFields();
            validName = validate.nameCheck(view.jtfName.getText());
            if(validName)
            {
                count++;
                validPhone = validate.validatePhone(view.jtfPhone.getText());
            }
            if(validPhone)
            {
                count++;
                validCard = validate.hasText(card.getCardNumber(view.jtfCardNumber));
            }

            if(validCard)
                count++;


            //if(count < 3)
                //displayError();
            else
            {
                destinationSelected = destination.getRadioButtonValue(view.jrbChicago,view.jrbNewYork, view.jrbSeattle, view.jrbSanFransisco);
                String cardNumber = card.getCardNumber(view.jtfCardNumber);
                String name = destination.getCustName(view.jtfName);
                String phone = destination.getPhoneNum(view.jtfPhone);
                displayMessage(cardNumber, name, phone);

            }   
        }

        private void displayError() {
            // TODO Auto-generated method stub

            StringBuilder sb = new StringBuilder();

            sb.append("Oops sorry, you typed something wrong");
            JOptionPane.showMessageDialog(null, sb);

        }

        public void displayMessage(String cardNumber, String name, String phone){
            // TODO Auto-generated method stub

            StringBuilder sb = new StringBuilder();
            NumberFormat f = NumberFormat.getCurrencyInstance();


            sb.append("Hello! Here is your estimate for the paint job:\n");
            sb.append("You chose the:" + cardNumber +name +phone );


            JOptionPane.showMessageDialog(null, sb);
        }
}
}

package edu.witc.TrainTicket.controller;

public class ValidateCard {
    public static boolean luhnVerify(String str) {
          int sum = 0;
          int value;
          int idx = str.length(); // Start from the end of string
          boolean alt = false;

          while(idx-- > 0) {
            // Get value. Throws error if it isn't a digit
            value = Integer.parseInt(str.substring(idx, idx + 1));
            if (alt) {
              value *= 2;
              if (value > 9) value -= 9;
            }
            sum += value;
            alt = !alt;  //Toggle alt-flag
          }
          return (sum % 10) == 0;
        }
}

package edu.witc.TrainTicket.controller;

import java.util.InputMismatchException;

import javax.swing.JRadioButton;

public class ValidateFields {
    private int count = 0;

public ValidateFields(){

    }

    public boolean hasText(String wallSpace){
        boolean isValid = false;
        count = 0;

        try
        {
            if(wallSpace.trim().length() > 0 && wallSpace.matches("[0-9]+"))
                count++;

            if(count == 1)
                return isValid = true;
        }
        catch(InputMismatchException e)
        {
            e.printStackTrace();
        }


        return isValid;
    }


    public boolean nameCheck(String name){
        boolean isValid = false;
        count = 0;

        try
        {
            if(name.trim().length() > 0 && name.matches("[a-zA-Z]+"))
                count++;

            if(count == 1)
                return isValid = true;
        }
        catch(InputMismatchException e)
        {
            e.printStackTrace();
        }


        return isValid;
    }

    public boolean validatePhone(String phone){
        boolean cleanPhone = false;
        count = 0;
        try
        {
            if(phone.replaceAll("\\D","").length() == 11 && phone.replaceAll("\\D","").matches("[0-9]+"))
                count++;

            if(count == 1)
                return cleanPhone = true;
        }
        catch(InputMismatchException e)
        {
            e.printStackTrace();
        }

        return cleanPhone;
    }
}

MODEL

    package edu.witc.TrainTicket.model;

    import javax.swing.JTextField;

    public class CreditCard {


    public CreditCard(){

    }

    public CreditCard(JTextField cardNumber){
    String cardNum = getCardNumber(cardNumber);
    }
        public String getCardNumber(JTextField cardNumber){
            return cardNumber.getText();
        }
    }

//This class sets gets all the information that the user has entered.
package edu.witc.TrainTicket.model;

import java.text.NumberFormat;

import javax.swing.JOptionPane;
import javax.swing.JRadioButton;
import javax.swing.JTextField;

import edu.witc.TrainTicket.view.*;

public class Destination {


    //Constructor with no arguments
    public Destination() {
    }


    public Destination(JRadioButton chicago, JRadioButton newYork, JRadioButton seattle, JRadioButton sanFransisco, JTextField name, JTextField phone) {

        String selectedDestination = getRadioButtonValue(chicago, newYork, seattle, sanFransisco);
        String custName = getCustName(name);
        String custPhone = getPhoneNum(phone);

        }


    public String getCustName(JTextField name){
        return name.getText();
    }

    public String getPhoneNum(JTextField phone){
        return phone.getText();
    }


    //get the type of paint
    public String getRadioButtonValue(JRadioButton chicago, JRadioButton newYork, JRadioButton seattle, JRadioButton sanFransisco) {

        String selected = "";

        if(chicago.isSelected())
            selected = "Chicago";
        if(newYork.isSelected())
            selected = "New York";
        if(seattle.isSelected())
            selected=  "Seattle";
        if(sanFransisco.isSelected())
            selected=  "San Fransisco";

        //JOptionPane.showMessageDialog(null, selected);

        return selected;
    }
}

VIEW

package edu.witc.TrainTicket.view;

import java.awt.BorderLayout;
import java.awt.GridLayout;
import java.awt.event.ActionListener;

import javax.swing.ButtonGroup;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JOptionPane;
import javax.swing.JPanel;
import javax.swing.JRadioButton;
import javax.swing.JTextField;

public class MainForm extends JFrame{

    /**
     * 
     */
    private static final long serialVersionUID = 1L;


    public JTextField jtfCardNumber = null;
    public JTextField jtfName = null;
    public JTextField jtfPhone = null;
    public JRadioButton jrbChicago = null;
    public JRadioButton jrbNewYork = null;
    public JRadioButton jrbSeattle = null;
    public JRadioButton jrbSanFransisco = null;

    protected JPanel radioPanel = null;
    protected JPanel jpLabels = null;
    protected JPanel jpTextFields = null;

    protected JButton jbtSubmit = null;

    public MainForm(){
        makeFrom();
    }

    private void makeFrom() {
        // TODO Auto-generated method stub

        jpLabels = new JPanel();
        jpTextFields = new JPanel();


        jtfCardNumber = new JTextField(16);
        jtfName = new JTextField();
        jtfPhone = new JTextField();
        jrbChicago = new JRadioButton("Chicago");
        jrbNewYork = new JRadioButton("New York");
        jrbSeattle = new JRadioButton("Seattle");
        jrbSanFransisco = new JRadioButton("San Fransisco");

        jrbChicago.setSelected(true);

        ButtonGroup group = new ButtonGroup();
        group.add(jrbChicago);
        group.add(jrbNewYork);
        group.add(jrbSeattle);
        group.add(jrbSanFransisco);

        jbtSubmit = new JButton("Submit");

        jpTextFields.setLayout(new GridLayout(7,1,1,1));
        jpLabels.setLayout(new GridLayout(7,1,1,1));

        jpLabels.add(new JLabel("Name:"));
        jpTextFields.add(jtfName);
        jpLabels.add(new JLabel("Phone:"));
        jpTextFields.add(jtfPhone);
        jpLabels.add(new JLabel("Credit Card Number"));
        jpTextFields.add(jtfCardNumber);
        jpLabels.add(new JLabel("Please Select A Destination"));
        jpLabels.add(jrbChicago);
        jpLabels.add(jrbNewYork);
        jpLabels.add(jrbSeattle);
        jpLabels.add(jrbSanFransisco);

        add(jpTextFields, BorderLayout.CENTER);
        add(jpLabels, BorderLayout.WEST);
        add(jbtSubmit, BorderLayout.SOUTH);
    }
    public void addSubmitListener(ActionListener click){
        jbtSubmit.addActionListener(click);
    }


    public void displayMessage(String message){
        JOptionPane.showMessageDialog(null, message);
    }
}

package edu.witc.TrainTicket.view;

import javax.swing.JFrame;

import edu.witc.TrainTicket.controller.*;
import edu.witc.TrainTicket.model.*;


public class Main {

    /**
     * @param args
     */
    public static void main(String[] args) {
        // TODO Auto-generated method stub

        MainForm view = new MainForm();
        Destination model = new Destination();
        CreditCard model1 = new CreditCard();

        @SuppressWarnings("unused")
        TrainTicketController controller = new TrainTicketController(view,model,model1);

        view.setTitle("Paint Job Information");
        view.setSize(500,300);
        view.setResizable(false);
        view.setLocationRelativeTo(null);
        view.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        view.setVisible(true);
    }
}
\$\endgroup\$
2
\$\begingroup\$

This methods should not be in a model public CreditCard(JTextField cardNumber) public Destination(JRadioButton chicago, JRadioButton newYork, JRadioButton seattle, JRadioButton sanFransisco, JTextField name, JTextField phone) Classes in model layers should be aware only of interface of underlying layers (DAOs or something similar). If it contains JTextField than you are telling your business model what view you are using. Send text from textfield directly in you service (model) methods. Or better send them some DTO wrapper object.

You misused ValidateCard and ValidateFields. ValidateFields belongs to the controller or view layer and ValidateCard belongs to the model layer and check the business requirements.

Also you duplicate the name of the cities. When you want to add new city you have to change the model and the view. Put them in one place (maybe something like CityRepository accessed by the Model layer classes) and get the cities from there.

So to recap Model is businees view. It should contain validation and perform your business task (in your case some traint icket processing) and should be aware only about data model. View should be aware only of controller and provide interface to interact with and methods how to get the data from the user Controller Drives the flow - it gets the data from view, wrap it in some business objects (like TrainTicket, User, etc...) and pass it to the model, takes output from the model and present it through the view.

\$\endgroup\$
  • \$\begingroup\$ Is there a better way to do all of this? Do you mind showing me an example of my program but better? Maybe comments would work. I wanna see a good version of what this would look like. \$\endgroup\$ – Justin Esders Oct 18 '13 at 20:49

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.