Student administration application

Question

I have set up a basic MVC project which is a student administration application based upon a CSV file as datastore. Each user has a specific role (student, lecturer, professor, leader of degree program). After successful login, the main window should have different actions to be performed based upon each role.

How can I hide the login view after successful login and show the main window depending on each user's role?

What about interfaces and abstract classes? Can I generalize/abstract the code better?

Regarding the different roles, I've thought about User as an abstract class:

• Student inherits from User
• Lecturer inherits from User
• Professor and Leader from Lecturer

Should the ActionListener be in the view or in the controller?

Is it better to init a frame in the main class or should each view extends/inherits from JFrame instead of JPanel?

General feedback on the code so far is very welcome.

users.csv

user,password,role
user1,password1,student
user2,password2,professor

Studentenverwaltung.java

package com.studentenverwaltung;

import java.awt.EventQueue;

import javax.swing.JFrame;

import com.studentenverwaltung.controller.UserController;
import com.studentenverwaltung.model.User;
import com.studentenverwaltung.view.LoginPanel;

class Studentenverwaltung implements Runnable {

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

    @Override
    public void run() {
        User user = new User();
        LoginPanel loginPanel = new LoginPanel(user);
        UserController userController = new UserController(user, loginPanel);

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

}

UserController.java

package com.studentenverwaltung.controller;

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

import com.studentenverwaltung.helpers.UserCredentials;
import com.studentenverwaltung.model.User;
import com.studentenverwaltung.persistence.FileUserDAO;
import com.studentenverwaltung.persistence.UserDAO;
import com.studentenverwaltung.view.LoginPanel;

public class UserController {
    private User user;
    private LoginPanel loginPanel;

    public UserController(User user, LoginPanel loginPanel) {
        this.user = user;
        this.loginPanel = loginPanel;

        this.loginPanel.login.addActionListener(new LoginListener());
    }

    class LoginListener implements ActionListener {

        @Override
        public void actionPerformed(ActionEvent e) {
            String id = UserController.this.loginPanel.getId();
            String password = UserController.this.loginPanel.getPassword();

            if (LoginListener.this.authenticate(new UserCredentials(id,
                    password))) {
                // show main window based upon specific user role and hide login
                // window
                System.out.println("Successfully logged in...");
            }

        }

        private boolean authenticate(UserCredentials userCredentials) {
            UserDAO userDAO = new FileUserDAO("Files/users.csv");

            String id = userCredentials.getId();
            String password = userCredentials.getPassword();

            if (userDAO.getUser(id) != null
                    && userDAO.getUser(id).checkPassword(password)) {
                return true;
            }

            return false;
        }

    }

}

User.java

package com.studentenverwaltung.model;

import java.util.Observable;

public class User extends Observable {
    private String id;
    private String password;
    private String role;

    public String getId() {
        return this.id;
    }

    public void setId(String id) {
        this.id = id;
        setChanged();
        notifyObservers();
    }

    public String getPassword() {
        return this.password;
    }

    public void setPassword(String password) {
        this.password = password;
        setChanged();
        notifyObservers();
    }

    public String getRole() {
        return this.role;
    }

    public void setRole(String role) {
        this.role = role;
        setChanged();
        notifyObservers();
    }

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

}

LoginPanel.java

package com.studentenverwaltung.view;

import java.awt.GridLayout;
import java.util.Observable;
import java.util.Observer;

import javax.swing.JButton;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JPasswordField;
import javax.swing.JTextField;

import com.studentenverwaltung.model.User;

public class LoginPanel extends JPanel {
    private static final long serialVersionUID = 1L;

    private User user;

    private JLabel idLbl;
    private JTextField idTxt;
    private JLabel pwdLbl;
    private JTextField pwdTxt;

    public JButton login;

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

        this.init();
    }

    private void init() {
        this.setLayout(new GridLayout(3, 1));

        this.idLbl = new JLabel("id");
        this.add(idLbl);

        this.idTxt = new JTextField(15);
        this.add(idTxt);

        this.pwdLbl = new JLabel("password");
        this.add(pwdLbl);

        this.pwdTxt = new JPasswordField(15);
        this.add(pwdTxt);

        this.login = new JButton("login");
        // login.addActionListener(new LoginListener());
        this.add(login);
    }

    public String getId() {
        return this.idTxt.getText();
    }

    public String getPassword() {
        return this.pwdTxt.getText();
    }

    private class UserObserver implements Observer {

        @Override
        public void update(Observable o, Object arg) {
            // TODO Auto-generated method stub

        }

    }

}

Answer 1

1. A great comment from @tb-'s answer:

   Do not extend JPanel, instead have a private JPanel attribute and deal with it (Encapsulation). There is no need to give access to all JPanel methods for code dealing with a UserPanel instance. If you extend, you are forced to stay with this forever, if you encapsulate, you can change whenever you want without taking care of something outside the class.

   I would make the serialVersionUID field superfluous.

2. I've never heard of Observable before. It might not be a coincidence, Péter Török's answer on SO has a good explanation about its disadvantages:

   They are not used, because their design is flawed: they are not type safe. You can attach any object that implements Observer to any Observable, which can result in subtle bugs down the line.

   [...]

3. Each user has a specific role (student, lecturer, professor, leader of degree program).

   I'd use composition instead (as you do with the role field). It's easier to extend (a professor also could be a lead, or a leader might not be a lecturer, for example).

   Check Effective Java, Second Edition, Item 16: Favor composition over inheritance if you haven't read it already.

4. public LoginPanel(User user) {
       this.user = user;
   }
   

   If User is null here I'd throw an exception instead. I guess it's rather a bug on the side of the caller and there is no point to continue the program with invalid state. You'll get an exception anyhow sooner or later. Throwing an exception immediately helps debugging a lot since you get a stacktrace with the frames of the erroneous client, not just a NullPointerException later, maybe from another thread. (The Pragmatic Programmer: From Journeyman to Master by Andrew Hunt and David Thomas: Dead Programs Tell No Lies.)

5. private JLabel idLbl;
   private JTextField idTxt;
   

   I'd avoid abbreviations like these field names. They make the code harder to read and undermine autocomplete. For example, if you type idLa autocomplete won't find anything. It's often annoying.

   Furthermore, they are hard to pronounce.

   If you can’t pronounce it, you can’t discuss it without sounding like an idiot. “Well, over here on the bee cee arr three cee enn tee we have a pee ess zee kyew int, see?” This matters because programming is a social activity.

   Source: Clean Code by Robert C. Martin, Use Pronounceable Names, p21

6. this.idLbl = new JLabel("id");
   

   There is no need of this. here. Modern IDEs use highlighting to separate local variables from instance variables.

7. if (userDAO.getUser(id) != null
           && userDAO.getUser(id).checkPassword(password)) {
       return true;
   }
   

   A local variable for User would be easier to read here:

   User user = userDAO.getUser(id);
   if (user != null && user.checkPassword(password)) {
       return true;
   }
   

8. I'd move the main method to a separate class. (For example, StudentAdministrationMain.) It's usually a good idea to separate a class from its clients.