Feedback on college project
$begingroup$
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());
}
}
java mvc
$endgroup$
add a comment |
$begingroup$
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());
}
}
java mvc
$endgroup$
$begingroup$
on popular naming convention is for all private variable to begin with an underscore idprivate JLabel _lblWelcome;
this would also eliminate the need for so many uses ofthis
.
$endgroup$
– Antarr Byrd
May 16 '13 at 20:45
7
$begingroup$
@atbyrd that would be true if this was C# code, not Java.
$endgroup$
– Jeff Vanzella
May 16 '13 at 21:07
3
$begingroup$
You are missing source comments....
$endgroup$
– Philip Helger
May 20 '13 at 19:56
$begingroup$
Indeed, if it's a college project, good javadoc will be appreciated by the teachers
$endgroup$
– K.C.
Dec 4 '13 at 8:20
add a comment |
$begingroup$
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());
}
}
java mvc
$endgroup$
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());
}
}
java mvc
java mvc
edited Nov 26 '13 at 22:45
Adam
4,95112344
4,95112344
asked May 16 '13 at 17:02
philk982philk982
9618
9618
$begingroup$
on popular naming convention is for all private variable to begin with an underscore idprivate JLabel _lblWelcome;
this would also eliminate the need for so many uses ofthis
.
$endgroup$
– Antarr Byrd
May 16 '13 at 20:45
7
$begingroup$
@atbyrd that would be true if this was C# code, not Java.
$endgroup$
– Jeff Vanzella
May 16 '13 at 21:07
3
$begingroup$
You are missing source comments....
$endgroup$
– Philip Helger
May 20 '13 at 19:56
$begingroup$
Indeed, if it's a college project, good javadoc will be appreciated by the teachers
$endgroup$
– K.C.
Dec 4 '13 at 8:20
add a comment |
$begingroup$
on popular naming convention is for all private variable to begin with an underscore idprivate JLabel _lblWelcome;
this would also eliminate the need for so many uses ofthis
.
$endgroup$
– Antarr Byrd
May 16 '13 at 20:45
7
$begingroup$
@atbyrd that would be true if this was C# code, not Java.
$endgroup$
– Jeff Vanzella
May 16 '13 at 21:07
3
$begingroup$
You are missing source comments....
$endgroup$
– Philip Helger
May 20 '13 at 19:56
$begingroup$
Indeed, if it's a college project, good javadoc will be appreciated by the teachers
$endgroup$
– K.C.
Dec 4 '13 at 8:20
$begingroup$
on popular naming convention is for all private variable to begin with an underscore id
private JLabel _lblWelcome;
this would also eliminate the need for so many uses of this
.$endgroup$
– Antarr Byrd
May 16 '13 at 20:45
$begingroup$
on popular naming convention is for all private variable to begin with an underscore id
private JLabel _lblWelcome;
this would also eliminate the need for so many uses of this
.$endgroup$
– Antarr Byrd
May 16 '13 at 20:45
7
7
$begingroup$
@atbyrd that would be true if this was C# code, not Java.
$endgroup$
– Jeff Vanzella
May 16 '13 at 21:07
$begingroup$
@atbyrd that would be true if this was C# code, not Java.
$endgroup$
– Jeff Vanzella
May 16 '13 at 21:07
3
3
$begingroup$
You are missing source comments....
$endgroup$
– Philip Helger
May 20 '13 at 19:56
$begingroup$
You are missing source comments....
$endgroup$
– Philip Helger
May 20 '13 at 19:56
$begingroup$
Indeed, if it's a college project, good javadoc will be appreciated by the teachers
$endgroup$
– K.C.
Dec 4 '13 at 8:20
$begingroup$
Indeed, if it's a college project, good javadoc will be appreciated by the teachers
$endgroup$
– K.C.
Dec 4 '13 at 8:20
add a comment |
2 Answers
2
active
oldest
votes
$begingroup$
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.
$endgroup$
add a comment |
$begingroup$
Bdheeejshbebi3db92xbiwbix2bxkg1xivwxviwxvwxviwxv8wivxwjvjvwxwvixivqxvwkxv2ivx29gx
New contributor
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f26252%2ffeedback-on-college-project%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
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.
$endgroup$
add a comment |
$begingroup$
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.
$endgroup$
add a comment |
$begingroup$
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.
$endgroup$
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.
edited May 23 '17 at 12:41
Community♦
1
1
answered Jan 8 '14 at 19:18
Dave JarvisDave Jarvis
1,6201926
1,6201926
add a comment |
add a comment |
$begingroup$
Bdheeejshbebi3db92xbiwbix2bxkg1xivwxviwxvwxviwxv8wivxwjvjvwxwvixivqxvwkxv2ivx29gx
New contributor
$endgroup$
add a comment |
$begingroup$
Bdheeejshbebi3db92xbiwbix2bxkg1xivwxviwxvwxviwxv8wivxwjvjvwxwvixivqxvwkxv2ivx29gx
New contributor
$endgroup$
add a comment |
$begingroup$
Bdheeejshbebi3db92xbiwbix2bxkg1xivwxviwxvwxviwxv8wivxwjvjvwxwvixivqxvwkxv2ivx29gx
New contributor
$endgroup$
Bdheeejshbebi3db92xbiwbix2bxkg1xivwxviwxvwxviwxv8wivxwjvjvwxwvixivqxvwkxv2ivx29gx
New contributor
New contributor
answered 23 mins ago
TarunTarun
1
1
New contributor
New contributor
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f26252%2ffeedback-on-college-project%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
$begingroup$
on popular naming convention is for all private variable to begin with an underscore id
private JLabel _lblWelcome;
this would also eliminate the need for so many uses ofthis
.$endgroup$
– Antarr Byrd
May 16 '13 at 20:45
7
$begingroup$
@atbyrd that would be true if this was C# code, not Java.
$endgroup$
– Jeff Vanzella
May 16 '13 at 21:07
3
$begingroup$
You are missing source comments....
$endgroup$
– Philip Helger
May 20 '13 at 19:56
$begingroup$
Indeed, if it's a college project, good javadoc will be appreciated by the teachers
$endgroup$
– K.C.
Dec 4 '13 at 8:20