Simple log-in to system

I am fairly new to the MVC paradigm and I am working with Swing at the moment. To test my understanding of MVC, I have written this simple program used to login in to a system. I was hoping someone could review what I have done so far and let me know if it follows MVC rules and best practices.

The View:

```
public class LoginScreen extends JFrame implements Observer {
private JLabel lblTitle, lblUsername, lblPassword;
private JTextField txtVanReg;
private JPasswordField txtPassword;
private JPanel pnlCenter, pnlNorth;
private JButton btnLogin, btnCancel;
private final Font fntOther = new Font("Verdana", Font.PLAIN, 16);
private final Font fntTitle = new Font("Verdana", Font.PLAIN, 20);
private LoginController controller;

public LoginScreen() {
this.controller = new LoginController(this);
this.setTitle("Login");
this.setLayout(new BorderLayout());

setUpComponents();
}

private void setUpComponents() {

lblTitle = new JLabel("Burrito Business");
lblUsername = new JLabel("Van reg: ");
lblPassword = new JLabel("Password: ");

txtVanReg = new JTextField();
txtPassword = new JPasswordField();

btnLogin = new JButton("Login");
btnCancel = new JButton("Cancel");

btnLogin.addActionListener(new ActionListener() {

public void actionPerformed(ActionEvent e) {
controller.loginRequested(txtVanReg.getText(), txtPassword.getPassword());
}
});

btnCancel.addActionListener(new ActionListener() {

public void actionPerformed(ActionEvent e) {
controller.loginCancelled();

}
});

pnlCenter = new JPanel(new GridLayout(3, 3));
pnlNorth = new JPanel(new BorderLayout());

pnlCenter.add(lblUsername);
pnlCenter.add(txtVanReg);
pnlCenter.add(lblPassword);
pnlCenter.add(txtPassword);
pnlCenter.add(btnLogin);
pnlCenter.add(btnCancel);

pnlNorth.add(lblTitle);
setFonts();

this.add(pnlNorth, BorderLayout.NORTH);
this.add(pnlCente

Thanks for sharing the code.

MVC

The basic concept of MVC is:

-
the model does not know of neither the controller nor the view .

It consists of data structure classes only. It should only contain logic to notify others about changes. It should define its own Listener interfaces and provide methods to register to change events. The Java-8 Property feature provides that out of the box.

Any logic to persist the model (to a database, the file system,a web service or alike) belongs to the controller layer.

-
the controller works on the model and does not know of the view.

This means your controller should not get the view as parameter and it should not create view related objects (the StaffMenu in particular).

Usually the view does not register for events at the controller. It reacts either on the value(s) returned by a method call or the Exception thrown.

-
the view displays values from the model and manipulates them through the controller.

This means the view registers itself for changes at the model and calls methods in the controller upon user input.


Your assertion that the Controller should persist the data is non-standard. See SoftwareEngineering.SE and Wikipedia for example. The Model is responsible for data persistence, either directly or by calling something else to handle it. At most, the Controller may trigger the shift to persistent storage. For example, by processing a "Save" button press and notifying the Model. – mdfst13

Some resources state that persistence layer and business logic are part of the (domain) model.

Usually the (data) Model has a very tight coupling to the persistence layer.

I.e.: There are lots of frameworks out there which allow us to generate DTO classes from a persistence model (XSD, JSON, database tables) or vice versa (by adding persistence specific annotations to the DTO classes).

This reduces the Controller layer to pure boilerplate code doing nothing else than translation user input to model actions (and vice versa). When looking at Swing the result would be that the Controller is completely integrated in the View represented by the many anonymous inner classes we register as listeners to Swings GUI components.

Other findings

Naming

Finding good names is the hardest part in programming. So always take your time to think about your identifier names.

On the bright side you follow the Java naming conventions. (except van_reg). This is a good starting point.

But you should have your method names start with a verb in its present tense.

E.g.: loginRequested should be named requestLogin.

Also you should use exact names.
Your variable dbConnection is of type DBUserAuth which not a database connection but uses a database connection itself.

Also: avoid single letter variable names or abbreviations.
There is no penalty for long names.
Its always better to be a bit "wordy" then being to short leaving room for interpretations.
But beware of writing "novels" on the other hand...

Wrong inheritance

We use inheritance if we want to change the behavior of a base class. This means we want to overwrite a base class's method or add a new one.

Your view extends JPanel without extending its behavior. You only configure it which could be done on a normal JFrame instance.

Dependency Injection / Inversion of Control

Your code uses some dependencies.
Eg.: your controller class uses an instance of the DBUserAuth class.

This instance is created in the constructor of the LoginController class.
This makes it hard to change the way your user authentication works.
What if you want to switch to tho a file based authentication, or to a web service?

You should pass the DBUserAuth instance as a parameter to the constructor.

Preferably this parameter would be declared as an interface that defines the methods called by the LoginController. The DBUserAuth class would then implement this interface.

SRP/SoC

Your violations of the MVC pattern are almost all consequences of the violation of the single responsibility pattern (SRP) and the separation of concerns pattern (SoC).

eg: Your method loginRequested does 2 things:

• call the user check in the database

• create the staffMenu

The latter is clearly a responsibility of the view.

StackExchange Code Review Q#161597, answer score: 4