Login window in Swing using MVP

Question

I am trying to grasp MVP (like so many) and although there are quite a few resources out there, I'm not sure I really get it.

I am trying to do this without frameworks to really see what's going on.

Here's my code:

main method (only method in a special class called Main):

SwingUtilities.invokeLater(() -> {
 JFrame mainWindow = new JFrame();
 mainWindow = new JFrame("MainFenster");
 mainWindow.setSize(500, 500);
 mainWindow.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
 mainWindow.setLocationRelativeTo(null);
 mainWindow.setLayout(new BorderLayout());
 LoginView loginView = new SwingLoginView(mainWindow);
 LoginModel loginModel = new LoginModelImpl();
 LoginService loginService = new LoginServiceImpl(loginModel);
 LoginPresenter presenter = new LoginPresenter(loginView, loginService);
 loginView.setPresenter(presenter);
 mainWindow.setVisible(true);
});

View interfaces

public interface View {
// Not sure what woudl go here, but just in case...
}
public interface LoginView extends View {
 void setErrorMessage(String errorMessage);
 void setNotificationMessage(String message);
 void navigateToHome(); // Should go here?
 void setPresenter(LoginPresenter presenter);
 interface LoginViewEventListener {
 void loginButtonClicked(String username, String password);
 }
}
public class SwingLoginView implements LoginView {
 private JFrame mainFrame;
 private LoginPresenter presenter; 
 private JTextArea errorMessage;
 private JTextArea password;
 private JTextArea username;
 private JTextArea notificationMessage;
 private JPanel panel;
 public SwingLoginView(JFrame mainWindow) {
 this.mainFrame = mainWindow;
 inititialize();
 }
 private void inititialize() {
 initializeComponents();
 }
 private void initializeComponents() {
 errorMessage = new JTextArea();
 errorMessage.setText("Hello");
 notificationMessage = new JTextArea();
 notificationMessage.setText("UU");
 password = new JTextArea();
 password.setText("password");
 username = new JTextArea();
 username.setText("username");
 JButton loginButton = new JButton("Press");
 loginButton.addActionListener((e) -> {
 // Could validation go here?
 presenter.loginButtonClicked(username.getText(), password.getText());
 });
 panel = new JPanel();
 panel.add(loginButton);
 panel.add(errorMessage);
 panel.add(notificationMessage);
 panel.add(password);
 panel.add(username);
 mainFrame.add(panel, BorderLayout.NORTH);
 }
 @Override
 public void setErrorMessage(String errorMessage) {
 this.errorMessage.setText(errorMessage);
 }
 @Override
 public void setNotificationMessage(String message) {
 this.notificationMessage.setText(message);
 }
 @Override
 public void navigateToHome() {
 // TODO Auto-generated method stub
 }
 @Override
 public void setPresenter(LoginPresenter presenter) {
 this.presenter = presenter;
 }
}

Presenter interface and class

public class LoginPresenter implements Presenter, LoginView.LoginViewEventListener {
private LoginView referenceToLoginView;
private LoginService referenceToLoginService;
public LoginPresenter(LoginView referenceToLoginView, LoginService loginService) {
 this.referenceToLoginView = referenceToLoginView;
 this.referenceToLoginService = loginService;
}
@Override
public void loginButtonClicked(String username, String password) {
 System.out.println("presenter called");
 if (username.equals("") || password.equals("")) {
 this.referenceToLoginView.setErrorMessage("Password and or username empty");
 return; // OK?
 }
 LoginModel loginModel = referenceToLoginService.login(username, password);
 referenceToLoginView.setNotificationMessage("NEU");
}
@Override
public View getView() {
 return referenceToLoginView;
}
}

and the model just has getters and setters for the username and password

And here's an attempt at a unit test:

@RunWith(MockitoJUnitRunner.class)
public class LoginTest {
 @Mock
 private LoginView mockLoginView;
 @Mock
 private LoginService mockLoginService;
 private LoginPresenter loginPresenter;
 @Before
 public void setUp() {
 loginPresenter = new LoginPresenter(mockLoginView, mockLoginService);
 mockLoginView.setPresenter(loginPresenter);
 }
 @Test
 public void loginShouldFailIfCredentialsAreEmpty() {
 loginPresenter.loginButtonClicked("", "");
 Mockito.verify(mockLoginView).setErrorMessage("Password and or username empty");
 }
}

So... is this going in the right direction?

Some more concrete questions are:

1. Should the view provide getter methods? Or should it pass the values to the presenter?
2. Should the view interface have a navigation method? Or should navigation be totally controlled by the presenter?
3. All my views need a reference to the main window / root pane or whatever, right?
4. Can I include small validity checks (empty username/password etc) in the view? Or should this go in the presenter, like in my case?
5. Should the presenter have knowledge of the model or should there be a service layer, like in my code? So should the login-method of my service return just a boolean to check if the login failed or a model-instance, like in my code?
6. What is meant by "the model is the business logic"? I thought the model was merely the java representation of a database table?

This is really tough for me to grasp.

Answer 1

Following are some improvements in your code. Implementing them is your choice:

1. In the class SwingLoginView, you may change private JTextArea password; to private JPasswordField password;. If you do that, change password.getText() to String.valueOf(password.getPassword()), because getText method in JPasswordField is deprecated.
2. Then, you may change private JTextArea username to private JTextField username because username is single-lined and JTextField is better for it.
3. In the SwingLoginView constructor, you are calling initialize method which is ONLY calling initializeComponents method. So you better call initializeComponents directly instead of calling initialize and delete/remove the method initialize from your code.
4. In the SwingUtilities.invokeLater(), change JFrame mainWindow = new JFrame(); to JFrame mainWindow = new JFrame("MainFenster"); and delete the next line. This will shorten your code.
5. If you are implementing point number 1 and 2, change password = new JTextArea(); to password = new JPasswordField("password"); and delete the next line. And change username = new JTextArea(); to username = new JTextField("username"); and delete the next line. This will again shorten your code.

• user3629892: Thanks for the suggestions! Any comments/tips regarding the architecture itself?
• Deepesh Choudhary: I have edited my answer. See point number 5. That's one more improvement, otherwise your code is perfect.

Answer 2

My two cents: In general, there are... some different interpretations of the MVP, if you google that pattern (also different names, just to mess with people); it also looks like, I have a different interpretation than you. my understanding is:

• The Model holds the state (username, password), it provides everything to make the the View run, it is only called by the presenter and does not know View or Presenter.
• View does not know Model and Presenter, the interface decouples a specifig gui implementation
• The presenter, which is like a controller, connects the two parts.

And another thing: I actually researched like all the gui patterns over several days and was so brain-pooped, because some patterns are so similar have different names, sometimes even in different programming languages, I still need a holiday xD

How you interpret the patterns isn't ... that big of a deal? It's more important to focus on what those patterns want to achieve, like make the view part as stupid as possible, exchangeability, and so on.

With that said / with that interpretation

• The LoginService needs the LoginModel, makes the LoginService not exchangeable. The service is 'one tier' below the LoginModel.
• The view has 'setErrorMessage' method, but if you consider, that you want to show messages in something like a messageLog, 'setErrorMessage' is very specific. Nitpicking, but if you think about it... : the interface view should not be bound to a specific implementation, not only technology, also it should not be bound to how it's implemented. That sounds very obvious (you know, interface is not the implementation), but actually sometimes very difficult.
• loginButtonClicked:
  • The username and password should be passed to the model
  • The login should be performed in the model (and model calls LoginService)
  • If a login can be performed, should be determined in the model, too
  • So you do something like if(!model.loginEnabled()) {} else if(model.loginOkay) {} else {}
• I wouldn't use 'reference' as prefix... it doesn't really help.
• 'loginShouldFailIfCredentialsAreEmpty() -> a bit shorter: loginFailsWithEmptyCredentials. The 'should' is not necessary. And 'must' would be better anyway :P
• The test itself is very clear - imo because of the mvp pattern, it's so easy and so much fun to test -, an empty line between the two, to split the 'when' from the 'then' block isn't big of a deal, but imo a pattern a dev should get used to, it can help a lot, if a test grows in lines. And: I would use anyString() for the error message parameter, or use a constant to avoid breaking your test, when you change the message.

To your questions:

1. I think both is possible and both is not a 'bad practice'. But consider: If you pass it from the view to the presenter, you might want to get more information from the View one day, (e.g. language?). I think it's more maintainable to get the information you actually need...
2. I honestly don't know, you can start wars about that topic. If you think about humble views, as in humble objects, the navigation itself shouldn't be within your 'component' (LoginComponent) at all, except maybe navigation within that component, but then it's not that humble anymore.
3. Same as 2... in my opinion: better not.
4. Nuuooooooo, View stupid, View not do many thing.
5. Haha, one would think so... maybe, maybe not, who knows...

I really hope that helps, but I'm afraid it does the opposite...