-3

Yo, I want to program a simple Messenger (with out Network and something) to practice my GUI - knowledge. However I encountered an issuer relating to the update of the Accounts.

The program have 4 (for the issue important) classes. Start, Register, Login and AccountList. Start just launches Register and Login. In Register you can add values to an static ArrayList in AccountList. Login compares the ArrayList with inputs. AccountList holds and static ArrayList of AccountData (another class).

The Problem now is, that the in Register created Accounts(ArrayList "Values") can't be used in Login. The Program creates in both of the classes an object of the class Account List.

...
AccountList list;

public Register()
{
    list = new AccountList();
...

Since the Login class will be called after the Register class, should the static ArrayList be updated?

here is the code of the four classes:

class Start

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

public class Start extends JFrame implements ActionListener
{

JButton bRegister;
JButton bLogin;

public static void main(String[] args)
{
    Start start = new Start();
}

   public Start()
   {
    setSize(400,400);
    setResizable(true);
    setVisible(true);
    setTitle("Start");
    setDefaultCloseOperation(EXIT_ON_CLOSE);

    FlowLayout mgr = new FlowLayout();
    setLayout(mgr);
    Container cp = getContentPane();

    bRegister = new JButton("Register");
    bRegister.addActionListener(this);
    cp.add(bRegister);

    bLogin = new JButton("Log in");
    bLogin.addActionListener(this);
    cp.add(bLogin);

}

public void reopen(){
    setVisible(true);
}

public void actionPerformed (ActionEvent evt)
{
    if (evt.getSource() == bRegister){
        Register r = new Register();

        setVisible(false);
    }
    else if(evt.getSource() == bLogin){

        Login l = new Login();

        setVisible(false);
    }
}

}

class Register:

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

public class Register extends JFrame implements ActionListener
{

JLabel lName; 
JTextField tName;

JLabel lPassword;
JPasswordField tPassword;

JButton addAcc;

AccountList list;

public Register()
{
    list = new AccountList();
    Create();
}

private void Create()
{
    setSize(400,400);
    setResizable(true);
    setVisible(true);

    setTitle("Register");
    setDefaultCloseOperation(EXIT_ON_CLOSE);

    FlowLayout mgr = new FlowLayout();
    setLayout(mgr);
    Container cp = getContentPane();

    lName = new JLabel("Name:");
    cp.add(lName);

    tName = new JTextField(20);
    cp.add(tName);

    lPassword = new JLabel("Password:");
    cp.add(lPassword);

    tPassword = new JPasswordField(20);
    cp.add(tPassword);

    addAcc = new JButton("Confirm");
    addAcc.addActionListener(this);
    cp.add(addAcc);
}

public void actionPerformed (ActionEvent evt)
{
    if (evt.getSource() == addAcc){

        String name = tName.getText();
        String password = new String(tPassword.getPassword());

        list.addAccount(name, password);

        setVisible(false);
        dispose();


        Start start = new Start();

    }
}

class Login:

}

import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;

public class Login extends JFrame implements ActionListener
{

JLabel lName; 
JTextField tName;

JLabel lPassword;
JPasswordField tPassword;

JButton confirm;

AccountList list;   

public Login(){
    list = new AccountList();
    Create();
}

private void Create()
{
    setSize(400,400);
    setResizable(true);
    setVisible(true);

    setTitle("Login");
    setDefaultCloseOperation(EXIT_ON_CLOSE);

    FlowLayout mgr = new FlowLayout();
    setLayout(mgr);
    Container cp = getContentPane();

    lName = new JLabel("Name:");
    cp.add(lName);

    tName = new JTextField(20);
    cp.add(tName);

    lPassword = new JLabel("Password:");
    cp.add(lPassword);

    tPassword = new JPasswordField(20);
    cp.add(tPassword);

    confirm = new JButton("Confirm");
    confirm.addActionListener(this);
    cp.add(confirm);

    list.print();
}

public void actionPerformed (ActionEvent evt)
{
    if (evt.getSource() == confirm){

        String inName = tName.getText();
        String inPassword = new String(tPassword.getPassword());
        AccountData user = null;

        for (AccountData acc : list.list){
            if (inName.equals(acc.getName())){
                user = acc;
            }
        }

        if (user != null){
            if (inPassword != user.getPassword()){
                System.out.println("go to inbox");//to change
            }
            else{
                JOptionPane.showMessageDialog(null, "Check the Password", "Wrong Password", JOptionPane.INFORMATION_MESSAGE);
            }
        }
        else{
            JOptionPane.showMessageDialog(null, "Check the Name", "Missing Name", JOptionPane.INFORMATION_MESSAGE);
        }

    }
}
}

class AccountList

import java.util.*;

public class AccountList
{

public static ArrayList<AccountData> list;

public AccountList()
{
    list = new ArrayList<AccountData>();
}

public void addAccount(String name, String password)
{
    AccountData nAcc = new AccountData(name , password);
    list.add(nAcc);

}

public void print(){
    System.out.println("on");
    for (AccountData acc: list){
        System.out.println(acc.getName());
    }

}

}
  • How about creating the list in `Start` and passing it into both `Register` and `Login` constructors? – shmosel Jun 15 '17 at 22:22
  • Yea its a way to solve it. However there should be a way to solve it this way. I mean, its the meaning behind static. – Julian Heinisch Jun 15 '17 at 22:26
  • 2
    The problem is you're replacing the static list in the constructor. Either way, it's not good practice to share data statically, especially not through an instance. It makes it unintuitive, difficult to maintain and error prone. – shmosel Jun 15 '17 at 22:32
  • 1
    @JulianHeinisch: In Java, `static` simply means "without any notion of or relationship to any instance of an object." As a general rule, using static variables to represent the state of a system is a spectacularly bad idea. – scottb Jun 15 '17 at 22:35
  • You're trying to implement the [evil *Singelton Pattern*](https://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons) and fail doing so. – Timothy Truckle Jun 15 '17 at 22:35
  • Thanks for the help. I will also hear to your advise and create the list in start. – Julian Heinisch Jun 15 '17 at 22:37
  • Besides the main point, you need to start the GUI in the Event Dispatch Thread, and you really should adhere to the Java naming conventions. You also should `setVisible(true)` after construction of the GUI, not in the middle of it, and in general avoid doing anything other than initialization in a constructor. Business logic should only happen after construction completes, especially in multi-threaded code like any Swing program. – Lew Bloch Jun 15 '17 at 22:46

1 Answers1

2

The fundamental problem with your solution is that each instance of AccountList replaces any preexisting value of 'list' on construction.

The simplest 'fix' would simply be to move the construction of the list to the definition of 'list'.

public static ArrayList<AccountData> list = new ArrayList<AccountData>();

That way all instances of AccountList would share the same ArrayList object.

However, it seems that your vision for AccountList is as a singleton. I'd suggest you read up on how to properly create a Singleton class.

Finally, Singletons make it difficult to write proper unit tests for client classes. I'd recommend no statics, no singletons, and switch to an IOC solution such as Spring.

assylias
  • 321,522
  • 82
  • 660
  • 783
Tom Drake
  • 527
  • 5
  • 11