3
\$\begingroup\$

I am currently learning Java programing by building real life business application for repair shops. I am posting this code so I can get critical review, and change my application architecture if necessary. I would like your opinion on my approach to MVC pattern.

This is a GUI swing class for one of the dialogs.

public class ClientRegistrationWindow

{   public JDialog window;
    public JPanel contentPane = new JPanel();
    public JLabel lablelIDValue = new JLabel();
    public JLabel labelFirstName = new JLabel();
    public JTextField textFieldFirstName = new JTextField();
    public JLabel labelLastName = new JLabel(); 
    public JTextField textFieldLastName = new JTextField();
    public JLabel labelPrimePhoneNum =  new JLabel();
    public JTextField textFieldPrimePhoneNum = new JTextField();
    public JTextField textFieldAlternativePhoneNum = new JTextField();
    public JTextField textFieldEmail = new JTextField();
    public JTextField textFieldAddress = new JTextField();
    public JLabel labelMarketing =  new JLabel();
    public JComboBox<Property> comboBoxMarketing = new JComboBox<Property>();
    public JButton buttonAdd = new JButton();
    public JButton buttonCancel = new JButton();

    /**
     * Creates JDialog "Customer Registration Form".
     */
    public ClientRegistrationWindow(Window owner)
    {
        createWindow(owner);
        createIDView();
        createFirstNameView();
        createLastNameView();
        createPrimePhoneNumView();
        createAlternativePhoneNumView();
        createEmailView();
        createAddressView();
        createMarketingView();
        createButtonsView();
    }

    private void createWindow(Window owner)
    {
        window = new JDialog(owner);
        window.setTitle(ClientGUITextUtils.TITLE);
        window.setResizable(false);
        window.setDefaultCloseOperation(JDialog.DISPOSE_ON_CLOSE);
        window.setBounds(100, 100, 460, 440);
        window.setContentPane(contentPane);
        window.setVisible(true);

        contentPane.setBorder(new EmptyBorder(5, 5, 5, 5));
        contentPane.setLayout(null);
    }

    private void createIDView()
    {
        JLabel labelID =  new JLabel();
        labelID.setText(ClientGUITextUtils.CLIENT_ID_LABEL);
        labelID.setBounds(35, 25, 110, 25);
        contentPane.add(labelID);

        lablelIDValue.setText("X-XXXXX");
        lablelIDValue.setHorizontalAlignment(SwingConstants.CENTER);
        lablelIDValue.setFont(new Font("Tahoma", Font.BOLD, 15));
        lablelIDValue.setBounds(150, 25, 160, 25);
        contentPane.add(lablelIDValue);
    }

    private void createFirstNameView()
    {
        labelFirstName.setText(ClientGUITextUtils.FIRST_NAME_LABEL);
        labelFirstName.setBounds(35, 90, 185, 15);
        contentPane.add(labelFirstName);

        textFieldFirstName.setColumns(10);
        textFieldFirstName.setBounds(35, 105, 185, 25);
        contentPane.add(textFieldFirstName);
    }

    private void createLastNameView()
    {
        labelLastName.setText(ClientGUITextUtils.LAST_NAME_LABEL);
        labelLastName.setBounds(235, 90, 185, 15);
        contentPane.add(labelLastName);

        textFieldLastName.setColumns(10);
        textFieldLastName.setBounds(235, 105, 185, 25);
        contentPane.add(textFieldLastName);
    }

    private void createPrimePhoneNumView()
    {
        labelPrimePhoneNum.setText(ClientGUITextUtils.PRIME_PHONE_NUMBER_LABEL);
        labelPrimePhoneNum.setBounds(35, 140, 185, 15);
        contentPane.add(labelPrimePhoneNum);

        textFieldPrimePhoneNum.setColumns(10);
        textFieldPrimePhoneNum.setBounds(35, 155, 185, 25);
        contentPane.add(textFieldPrimePhoneNum);
    }

    private void createAlternativePhoneNumView()
    {
        JLabel labelAlternativePhoneNum =new JLabel(ClientGUITextUtils.SECOND_NUMBER_LABEL);
        labelAlternativePhoneNum.setBounds(235, 140, 185, 15);
        contentPane.add(labelAlternativePhoneNum);

        textFieldAlternativePhoneNum.setColumns(10);
        textFieldAlternativePhoneNum.setBounds(235, 155, 185, 25);
        contentPane.add(textFieldAlternativePhoneNum);
    }

    private void createEmailView()
    {
        JLabel labelEmail = new JLabel(ClientGUITextUtils.EMAIL_LABEL);
        labelEmail.setBounds(35, 190, 385, 15);
        contentPane.add(labelEmail);

        textFieldEmail.setColumns(10);
        textFieldEmail.setBounds(35, 205, 385, 25);
        contentPane.add(textFieldEmail);
    }

    private void createAddressView()
    {
        JLabel labelAddress =  new JLabel(ClientGUITextUtils.ADDRESS_LABEL);
        labelAddress.setBounds(35, 240, 385, 15);
        contentPane.add(labelAddress);

        textFieldAddress.setColumns(10);
        textFieldAddress.setBounds(35, 255, 385, 25);
        contentPane.add(textFieldAddress);
    }

    private void createMarketingView()
    {
        labelMarketing.setText(ClientGUITextUtils.MARKETING_LABEL);
        labelMarketing.setBounds(235, 290, 185, 15);
        contentPane.add(labelMarketing);

        comboBoxMarketing.setSize(185, 25);
        comboBoxMarketing.setLocation(235, 305);
        contentPane.add(comboBoxMarketing);
    }

    private void createButtonsView()
    {
        buttonAdd.setText(ClientGUITextUtils.ADD_CLIENT_BUTTON);
        buttonAdd.setFont(new Font("Dialog", Font.PLAIN, 15));
        buttonAdd.setBounds(35, 365, 110, 25);
        contentPane.add(buttonAdd);

        buttonCancel.setText(ClientGUITextUtils.CANCEL_BUTTON);
        buttonCancel.setFont(new Font("Dialog", Font.PLAIN, 15));
        buttonCancel.setBounds(310, 365, 110, 25);
        contentPane.add(buttonCancel);
    }
}

This is the controller class for that window.

public class ClientRegistrationController implements InputDialogController
{
    private int clientID;
    private ClientRegistrationWindow clientRegistration;

    public ClientRegistrationController(Window owner)
    {
        clientID = IDGeneratorUtils.getNewClientID();

        clientRegistration = new ClientRegistrationWindow(owner);
        clientRegistration.lablelIDValue.setText(String.valueOf(clientID));
        clientRegistration.buttonAdd.addActionListener(new AddDataElement(this));
        clientRegistration.buttonCancel.addActionListener(new CloseWindow(this));
    }

    private Client scanWindowInput()
    {
        Client newClient = new Client();
        newClient.setID(clientID);
        newClient.setFirstName(clientRegistration.textFieldFirstName.getText());
        newClient.setLastName(clientRegistration.textFieldLastName.getText());
        newClient.setPrimePhoneNumber(clientRegistration.textFieldPrimePhoneNum.getText());
        newClient.setAlternativePhoneNumber(clientRegistration.textFieldAlternativePhoneNum.getText());
        newClient.setEmail(clientRegistration.textFieldEmail.getText());
        newClient.setAddress(clientRegistration.textFieldAddress.getText());
        newClient.setMarketing((Property) clientRegistration.comboBoxMarketing.getSelectedItem());

        return newClient;
    }

    @Override
    public void trySavingDataElement()
    {
        if(ClientValidatorUtils.isValid(clientRegistration))
        {
            DataStructure.clientsDataTable.save(scanWindowInput());
            IDGeneratorUtils.incrementClientCounter();
            closeWindow();
        }
        else
        {
            new ShowClientInputErrors(clientRegistration);
        }
    }

    @Override
    public void closeWindow()
    {
        clientRegistration.window.dispose();
    }
}

This is Client registration input validation class.

public class ClientValidatorUtils
{
    public static boolean isValid(ClientRegistrationWindow clientRegistration)
    {
        return isFirstNameEntered(clientRegistration)
            && isLastNameEntered(clientRegistration)
            && isPhoneNumberEntered(clientRegistration)
            && isPhoneNumberUnique(clientRegistration)
            && isMarketingNotSelected(clientRegistration);
    }

    public static boolean isFirstNameEntered(ClientRegistrationWindow clientRegistration)
    {
        return !("".equals(clientRegistration.textFieldFirstName.getText()));
    }

    public static boolean isLastNameEntered(ClientRegistrationWindow clientRegistration)
    {
        return !("".equals(clientRegistration.textFieldLastName.getText()));
    }

    public static boolean isPhoneNumberEntered(ClientRegistrationWindow clientRegistration)
    {
        return !("".equals(clientRegistration.textFieldPrimePhoneNum.getText()));
    }

    public static boolean isPhoneNumberUnique(ClientRegistrationWindow clientRegistration)
    {
        return !(DataStructure.clientsDataTable
                .uniqueStringCollision(clientRegistration.textFieldPrimePhoneNum.getText()));
    }

    public static boolean isMarketingNotSelected(ClientRegistrationWindow clientRegistration)
    {
        return clientRegistration.comboBoxMarketing.getSelectedItem() != null;
    }
}

This is the input error handler.

public class ShowClientInputErrors
{
    private ClientRegistrationWindow clientRegistration;

    public ShowClientInputErrors(ClientRegistrationWindow clientRegistration)
    {
        this.clientRegistration = clientRegistration;
        showPhomeNumberNotUniqueError();
        showPhomeNumberNotEnterdError();
        showFirstNameNotEnteredError();
        showLastNameNotEnteredError();
        showMarketingNotSelectedError();
    }

    private void showPhomeNumberNotUniqueError()
    {
        if(ClientValidatorUtils.isPhoneNumberUnique(clientRegistration))
        {
            displayDefaultPhoneNumber();
        }
        else
        {
            displayPhoneNumberDuplicateError();
        }
    }

    private void showPhomeNumberNotEnterdError()
    {
        if(ClientValidatorUtils.isPhoneNumberEntered(clientRegistration))
        {
            displayDefaultPhoneNumber();
        }
        else
        {
            displayPhoneNumberNotEnteredError();
        }
    }

    private void displayDefaultPhoneNumber()
    {
        clientRegistration.labelPrimePhoneNum
        .setText(ClientGUITextUtils.PRIME_PHONE_NUMBER_LABEL);

        clientRegistration.textFieldPrimePhoneNum.setBackground(Color.WHITE);
    }

    private void displayPhoneNumberNotEnteredError()
    {
        clientRegistration.labelPrimePhoneNum
        .setText(ClientGUITextUtils.PHONE_NUMBER_NOT_ENTERED_ERROR_MESSAGE);

        clientRegistration.textFieldPrimePhoneNum.setBackground(Color.YELLOW);
    }

    private void displayPhoneNumberDuplicateError()
    {
        clientRegistration.labelPrimePhoneNum
        .setText(ClientGUITextUtils.PHONE_NUMBER_NOT_UNIQUE_ERROR_MESSAGE);

        clientRegistration.textFieldPrimePhoneNum.setBackground(Color.YELLOW);
    }

    private void showFirstNameNotEnteredError()
    {
        if(ClientValidatorUtils.isFirstNameEntered(clientRegistration))
        {
            clientRegistration.labelFirstName
            .setText(ClientGUITextUtils.FIRST_NAME_LABEL);

            clientRegistration.textFieldFirstName.setBackground(Color.WHITE);
        }
        else
        {
            clientRegistration.labelFirstName
            .setText(ClientGUITextUtils.FIRST_NAME_NOT_ENTERED_ERROR_MESSAGE);

            clientRegistration.textFieldFirstName.setBackground(Color.YELLOW);
        }
    }

    private void showLastNameNotEnteredError()
    {
        if(ClientValidatorUtils.isLastNameEntered(clientRegistration))
        {
            clientRegistration.labelLastName
            .setText(ClientGUITextUtils.LAST_NAME_LABEL);

            clientRegistration.textFieldLastName.setBackground(Color.WHITE);
        }
        else
        {
            clientRegistration.labelLastName
            .setText(ClientGUITextUtils.LAST_NAME_NOT_ENTERED_ERROR_MESSAGE);

            clientRegistration.textFieldLastName.setBackground(Color.YELLOW);
        }
    }

    private void showMarketingNotSelectedError()
    {
        if(ClientValidatorUtils.isMarketingNotSelected(clientRegistration))
        {
            clientRegistration.labelMarketing
            .setText(ClientGUITextUtils.MARKETING_LABEL);

            clientRegistration.comboBoxMarketing.setBackground(Color.WHITE);
        }
        else
        {
            clientRegistration.labelMarketing
            .setText(ClientGUITextUtils.MARKETING_NOT_SELECTED_ERROR_MESSAGE);

            clientRegistration.comboBoxMarketing.setBackground(Color.YELLOW);
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

I think I found even better MVC architecture for my application. I have used inheritance for my input dialog controller classes , and composition of Jpanels in my Swing dialog classes, to avoid code duplication, and to get more clear code. Or I hope so..

This is my abstract input dialog controller class that is a super class for all input dialog controllers. It is responsible for setting the ID label value, setting Add and Cancel buttons ActionListeners, and has the main function of input controllers, saving data.

public abstract class InputDialogController implements WindowController
{
    protected DataType dataType;
    protected int id;
    protected InputDialog gui;

    protected InputDialogController(WindowController owner, DataType dataType)
    {
        this.dataType = dataType;
        gui = InputDialogFactory.getWindow(owner.getWindow(), dataType);
        id = IDGenerator.getNewID(dataType);
        gui.getIdPanel().setIdValue(IDGenerator.formatRegularID(id));
        gui.getInputButtonPanel().setBtnAddActionListener(ActionListenerFactory.saveData(this));
        gui.getInputButtonPanel().setBtnCancelActionListener(ActionListenerFactory.closeWindow(this));
    }

    @Override
    public Window getWindow()
    {
        return (Window) gui;
    }

    public void trySavingDataElement()
    {
        if(isInputValid())
        {
            DataManager.save(createDataElement());
            getWindow().dispose();
        }
        else
        {
            showInputErrors();
        }
    }

    protected abstract boolean isInputValid();

    protected abstract DataElement createDataElement();

    protected abstract void showInputErrors();
}

This is one of mine concrete input dialog controllers, the ClientRegistrationController class.

public class ClientRegistrationController extends InputDialogController
{
    private ClientRegistrationDialog clientGUI;

    public ClientRegistrationController(WindowController owner, DataType dataType)
    {
        super(owner, dataType);
        clientGUI = (ClientRegistrationDialog) super.gui;
        clientGUI.getMarketingPanel().setMarketingCmbModel(CmbModelFactory.getModel(dataType));
        clientGUI.getMarketingPanel()
                 .setBtnMarketingActionListener(ActionListenerFactory
                 .openNewWindow(this, DataType.MARKETING_TYPE));

    }

    public void updateComboBoxes(String item)
    {
        clientGUI.getMarketingPanel().setMarketing(item);
    }

    @Override
    protected boolean isInputValid()
    {
        return isNameValid()
            && isPhoneNumberValid()
            && isMarketingSelected();
    }

    private boolean isNameValid( )
    {
        return !("".equals(clientGUI.getPersonalInfoPanel().getName()));
    }

    private boolean isPhoneNumberValid()
    {
        String phoneNumber = clientGUI.getPersonalInfoPanel().getPrimePhoneNumber();

        return !(DataManager.clientsDataTable.uniqueStringCollision(phoneNumber)
             || ("".equals(phoneNumber)));
    }

    private boolean isMarketingSelected()
    {
        return clientGUI.getMarketingPanel().getMarketing() != "";
    }

    @Override
    protected Client createDataElement()
    {
        Client newClient= new Client();

        newClient.setId(id);
        newClient.setName(clientGUI.getPersonalInfoPanel().getName());
        newClient.setPrimePhoneNumber(clientGUI.getPersonalInfoPanel().getPrimePhoneNumber());
        newClient.setAlternativePhoneNumber(clientGUI.getPersonalInfoPanel().getAltPoneNumber());
        newClient.setEmail(clientGUI.getPersonalInfoPanel().getEmail());
        newClient.setAddress(clientGUI.getPersonalInfoPanel().getAddress());
        newClient.setMarketing(DataElementGetter.getMarketing(clientGUI.getMarketingPanel().getMarketing()));

        return newClient;
    }

    @Override
    protected void showInputErrors()
    {
        checkName();
        checkPhoneNumber();
        checkMarketing();
    }

    private void checkName()
    {
        if(isNameValid())
        {
            clientGUI.getPersonalInfoPanel().showNameDefault();
        }
        else
        {
            clientGUI.getPersonalInfoPanel().showNameError();
        }
    }


    private void checkPhoneNumber()
    {

        if(isPhoneNumberValid())
        {
            clientGUI.getPersonalInfoPanel().showPhoneDefault();
        }
        else
        {
            clientGUI.getPersonalInfoPanel().showPhoneError();
        }
    }

    private void checkMarketing()
    {
        if(isMarketingSelected())
        {
            clientGUI.getMarketingPanel().showMarketingDefault();
        }
        else
        {
            clientGUI.getMarketingPanel().showMarketingError();
        }
    }
}

This is the swing class for client registration:

public class ClientRegistrationDialog extends JDialog implements InputDialog
{
    private static final long serialVersionUID = -394107433140693140L;
    private IdPanel idPanel = new IdPanel();
    private PersonalInfoPanel personalInfoPanel = new PersonalInfoPanel();
    private MarketingPanel marketingPanel = new MarketingPanel();
    private InputButtonPanel buttonPanel = new InputButtonPanel();

    public ClientRegistrationDialog(Window owner)
    {
        super(owner);
        getContentPane().setLayout(new MigLayout("", "[434px]", "[25px:n][][][]"));
        getContentPane().add(idPanel, "cell 0 0,grow");
        getContentPane().add(personalInfoPanel, "cell 0 1,grow");
        getContentPane().add(marketingPanel, "cell 0 2,grow");
        getContentPane().add(buttonPanel, "cell 0 3,grow");
    }

    @Override
    public IdPanel getIdPanel()
    {
        return idPanel;
    }

    public PersonalInfoPanel getPersonalInfoPanel()
    {
        return personalInfoPanel;
    }

    public MarketingPanel getMarketingPanel()
    {
        return marketingPanel;
    }

    @Override
    public InputButtonPanel getInputButtonPanel()
    {
        return buttonPanel;
    }
}

And this is one of it's panels, the PersonalInfoPanel.

public class PersonalInfoPanel extends JPanel
{
    private static final long serialVersionUID = -1636004925810635460L;
    private JTextField txtName;
    private JTextField txtPrimePhone;
    private JTextField txtAltPhone;
    private JTextField txtEmail;
    private JTextField txtAddress;

    public PersonalInfoPanel()
    {
        setLayout(new MigLayout("", "[][5.00][grow]", "[20px:n,fill][20px:n,fill][20px:n,fill][][20px:n]"));

        JLabel lblName = LabelFactory.createJLabel("First and Last Name", new Font("Tahoma", Font.PLAIN, 13));
        add(lblName, "cell 0 0,growy");

        txtName = TextFieldFactory.createJTextField(10);
        add(txtName, "cell 2 0,grow");

        JLabel lblPrimePhone = LabelFactory.createJLabel("Primary Phone Number", new Font("Tahoma", Font.PLAIN, 13));
        add(lblPrimePhone, "cell 0 1,growy");

        txtPrimePhone = TextFieldFactory.createJTextField(10);
        add(txtPrimePhone, "cell 2 1,grow");

        JLabel lblAltPhone = LabelFactory.createJLabel("Alternative Phone Number", new Font("Tahoma", Font.PLAIN, 13));
        add(lblAltPhone, "cell 0 2,growy");

        txtAltPhone = TextFieldFactory.createJTextField(10);
        add(txtAltPhone, "cell 2 2,grow");

        JLabel lblEmail = LabelFactory.createJLabel("Email Address", new Font("Tahoma", Font.PLAIN, 13));
        add(lblEmail, "cell 0 3,growy");

        txtEmail = TextFieldFactory.createJTextField(10);
        add(txtEmail, "cell 2 3,growx");

        JLabel lblHomeAddress = LabelFactory.createJLabel("Home Address", new Font("Tahoma", Font.PLAIN, 13));
        add(lblHomeAddress, "cell 0 4,growy");

        txtAddress = TextFieldFactory.createJTextField(10);
        add(txtAddress, "cell 2 4,grow");
    }

    public String getPersonName()
    {
        return txtName.getText();
    }

    public String getPrimePhoneNumber()
    {
        return txtPrimePhone.getText();
    }

    public String getAltPoneNumber()
    {
        return txtAltPhone.getText();
    }

    public String getEmail()
    {
        return txtAltPhone.getText();
    }

    public String getAddress()
    {
        return txtAddress.getText();
    }

    public void showNameDefault()
    {
        txtName.setBackground(Color.WHITE);
    }

    public void showNameError()
    {
        txtName.setBackground(Color.YELLOW);
    }

    public void showPhoneDefault()
    {
        txtName.setBackground(Color.WHITE);
    }

    public void showPhoneError()
    {
        txtName.setBackground(Color.YELLOW);
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Self-answers are perfectly fine, but this one reads like it's kind of just following-up on the OP.. If you would like feedback on this new updated code, it would be better to present it in a new question... but since there hasn't been any answers yet, you could simply edit the OP with your latest bestest version! Do that and your post will be bumped onto the home page. Add additional context/explanations, screenshots, tell us about what the app does and where that code fits in, ..and I'll put up a nice bounty to draw more attention ;-) \$\endgroup\$ – Mathieu Guindon 2 days ago

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.