Student administration application

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.Act

-
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.

-
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.


[...]

-

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.

-

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.)

-

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

-

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

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

-

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;
}

-
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.

StackExchange Code Review Q#26065, answer score: 6