Feedback on college project [closed]

Question

I would appreciate some feedback regarding best practices on the following code for a college project.

What should go in the controller and what in the views? How to attach and remove the views to the single main frame? Change the Controller after successful login?

Studentenverwaltung.java

package com.studentenverwaltung;

import com.studentenverwaltung.controller.LoginController;
import com.studentenverwaltung.model.User;
import com.studentenverwaltung.view.LoginView;

import javax.swing.*;
import java.awt.*;

class Studentenverwaltung implements Runnable {

    public static void main(String[] args) {
        EventQueue.invokeLater(new Studentenverwaltung());
    }

    @Override
    public void run() {
        User user = new User();
        LoginView loginView = new LoginView(user);
        LoginController loginController = new LoginController(user, loginView);

        JFrame frame = new JFrame();
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.add(loginView);
        frame.pack();
        frame.setLocationRelativeTo(null);
        frame.setVisible(true);
    }
}

LoginController.java

package com.studentenverwaltung.controller;

import com.studentenverwaltung.model.User;
import com.studentenverwaltung.view.LoginView;

public class LoginController {
    private User user;
    private LoginView LoginView;

    public LoginController(User user, LoginView LoginView) {
        this.user = user;
        this.LoginView = LoginView;
    }
}

User.java

package com.studentenverwaltung.model;

import java.util.Observable;

public class User extends Observable {
    private String lastName;
    private String firstName;
    private String role;
    private String id;
    private String password;
    private String degreeProgram;
    private boolean isLeaderOfDegreeProgram;
    private String course;

    // Getter & Setter

    public boolean checkPassword(String password) {
        return this.password.equals(password);
        // this.setChanged();
        // this.notifyObservers(this);
    }
}

LoginView.java

package com.studentenverwaltung.view;

import com.studentenverwaltung.model.User;
import com.studentenverwaltung.persistence.FileUserDAO;

import javax.swing.*;
import java.awt.event.*;
import java.util.Observable;
import java.util.Observer;

public class LoginView extends JDialog {
    private JPanel contentPane;
    private JButton btnLogin;
    private JButton btnCancel;
    private JTextField txtId;
    private JPasswordField txtPassword;
    private User user;

    public LoginView(User user) {
        this.user = user;
        this.user.addObserver(new UserObserver());

        this.init();
    }

    private void init() {
        this.setContentPane(contentPane);
        this.setModal(true);
        this.getRootPane().setDefaultButton(btnLogin);

        this.btnLogin.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                LoginView.this.onLogin();
            }
        });

        this.btnCancel.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                LoginView.this.onCancel();
            }
        });

        this.setDefaultCloseOperation(DO_NOTHING_ON_CLOSE);
        this.addWindowListener(new WindowAdapter() {
            public void windowClosing(WindowEvent e) {
                LoginView.this.onCancel();
            }
        });

        this.contentPane.registerKeyboardAction(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                LoginView.this.onCancel();
            }
        }, KeyStroke.getKeyStroke(KeyEvent.VK_ESCAPE, 0), JComponent.WHEN_ANCESTOR_OF_FOCUSED_COMPONENT);
    }

/*
public static void main(String[] args) {
LoginView dialog = new LoginView();
dialog.pack();
dialog.setVisible(true);
System.exit(0);
}
*/

    private void onLogin() {
        FileUserDAO userDAO;
        String id, password;
        User user;

        userDAO = new FileUserDAO("Files/stud_info.csv");
        id = this.txtId.getText();
        password = this.txtPassword.getText();
        user = userDAO.getUser(id);

        if (user != null && user.checkPassword(password)) {
            this.dispose();

            // switch (user.getRole()) {
            // case "student":
            // //
            // case "lecturer":
            // //
            // case "professor":
            // if (user.getIsLeaderOfDegreeProgram()) {
            // // leader
            // }
            //
            // // professor
            // }

            frame.add(new StudentView(user).contentPane);
            frame.pack();
            frame.setLocationRelativeTo(null);
            frame.setVisible(true);
        }
    }

    private void onCancel() {
        this.dispose();
    }

    private class UserObserver implements Observer {
        @Override
        public void update(Observable o, Object arg) {
            //To change body of implemented methods use File | Settings | File Templates.
        }
    }
}

StudentView.java

package com.studentenverwaltung.view;

import com.studentenverwaltung.model.User;

import javax.swing.*;

public class StudentView {
    public JPanel contentPane;
    private JLabel lblWelcome;
    private JButton btnChangePassword;
    private JButton btnLogout;
    private JTextField txtId;
    private JTextField txtPassword;
    private JTextField txtDegreeProgram;
    private JTable tblPerformance;
    private User user;

    public StudentView(User user) {
        this.user = user;
        // this.tblPerformance.setModel(this.user.getAllCourses());

        this.init();
    }

    private void init() {
        this.lblWelcome.setText("Herzlich Willkommen, " + this.user.getFirstName() + " " + this.user.getLastName());
        this.txtId.setText(this.user.getId());
        this.txtPassword.setText(this.user.getPassword());
        this.txtDegreeProgram.setText(this.user.getDegreeProgram());
    }
}

Answer 1

Use self-encapsulation, which will help apply the Open-Closed Principle.

Studentenverwaltung.java

Use an IDE that will automatically import classes explicitly. Future maintainers should not have to guess what classes are imported:

import javax.swing.*;
import java.awt.*;

Avoid the .*.

LoginController.java

The LoginController has no purpose. An object-oriented class must have both behaviour and attributes. Moreover, classes should be defined first in terms of behaviour.

Typically this means asking, "What does the LoginController do?" Then define those responsibilities as methods. Attributes (data) are a secondary consideration. This is more evident after reading "Tell, Don't Ask."

User.java

A few issues:

• Classes should be as generic as possible
• Passwords should not be stored in plaintext
• Password verification might be the LoginController's responsibility

A generic User, for example, would not have the following attributes:

private String password;
private String degreeProgram;
private boolean isLeaderOfDegreeProgram;
private String course;

Using a single role imposes an arbitrary limit without necessity. If the User can have multiple roles, then one of those roles can be "Degree Program Leader".

At the very least the password should be hashed.

LoginView.java

Stylistically, most of the references to this. are superfluous:

this.setContentPane(contentPane);
this.setModal(true);
this.getRootPane().setDefaultButton(btnLogin);

The following seems overly verbose:

LoginView.this.onLogin();

I think it can be:

onLogin();

StudentView.java

There is some duplication between LoginView and StudentView that can be abstracted:

public JPanel contentPane;
private JTextField txtId;
private JTextField txtPassword;
private User user;

These can be in a generic "View" superclass that contains elements common to both. Or perhaps they can be in a common class that is included by both.

Make all variables private. No exceptions.

StudentView.java

Avoid hardcoding text:

    this.lblWelcome.setText("Herzlich Willkommen, " + this.user.getFirstName() + " " + this.user.getLastName());

Use a ResourceBundle for compound messages.