Returning database id in a global variable. How to fix?

Question

After command insert, i can recover the id generated by the database using a global variable public static int returnID. But I didn’t think it was very cool to leave the variable static. Is there any other way to retrieve this information? Code below:

---

My canvas:

public class TelaPrincipal extends JFrame implements ActionListener {

    private static final long serialVersionUID = 1L;

    JPanel panelPrincipal,panelCenter,panelButton;
    JLabel idlbl,nomelbl,sexolbl,rglbl,cpflbl,dtnasclbl,dtcadlbl,estcivlbl;
    JTextField idField,nomeField,sexoField,rgField,cpfField,dtnascField,dtcadField,estcivField;
    JButton btnCad,btnLimpar,btnFechar;

    public TelaPrincipal(){
        UtilTelas.iniciarTelas(300, 300, "Cadastro de Pessoa", this);
        panelPrincipal = new JPanel(new BorderLayout());
        this.add(panelPrincipal);

        Center();
        Button();

        this.setVisible(true);
        pack();
    }

    private void Center(){      
        panelCenter = new JPanel(new FlowLayout(FlowLayout.LEFT));  
        JPanel gridLayout = new JPanel(new GridBagLayout());
        GridBagConstraints gbc = new GridBagConstraints();
        gbc.insets = new Insets(3, 3, 3, 3);

        int x = 0;
        int y = 0;
        int fieldX = 0;
        int fieldY = 0;

        idlbl = new JLabel("Código : ");
        gbc.gridx = x;
        gbc.gridy = ++y;
        gbc.anchor = GridBagConstraints.EAST;
        gridLayout.add(idlbl,gbc);

        nomelbl = new JLabel("Nome : ");
        gbc.gridx = x;
        gbc.gridy = ++y;
        gbc.anchor = GridBagConstraints.EAST;
        gridLayout.add(nomelbl,gbc);

        sexolbl = new JLabel("Sexo : ");
        gbc.gridx = x;
        gbc.gridy = ++y;
        gbc.anchor = GridBagConstraints.EAST;
        gridLayout.add(sexolbl,gbc);

        rglbl = new JLabel("RG : ");
        gbc.gridx = x;
        gbc.gridy = ++y;
        gbc.anchor = GridBagConstraints.EAST;
        gridLayout.add(rglbl,gbc);

        cpflbl = new JLabel("CPF : ");
        gbc.gridx = x;
        gbc.gridy = ++y;
        gbc.anchor = GridBagConstraints.EAST;
        gridLayout.add(cpflbl,gbc);

        dtnasclbl = new JLabel("Dt Nasci : ");
        gbc.gridx = x;
        gbc.gridy = ++y;
        gbc.anchor = GridBagConstraints.EAST;
        gridLayout.add(dtnasclbl,gbc);

        dtcadlbl = new JLabel("Dt Cadastro : ");
        gbc.gridx = x;
        gbc.gridy = ++y;
        gbc.anchor = GridBagConstraints.EAST;
        gridLayout.add(dtcadlbl,gbc);

        estcivlbl = new JLabel("Estado Civil");
        gbc.gridx = x;
        gbc.gridy = ++y;
        gbc.anchor = GridBagConstraints.EAST;
        gridLayout.add(estcivlbl,gbc);

        idField = new JTextField(5);
        gbc.gridx = ++fieldX;
        gbc.gridy = ++fieldY;
        gbc.anchor = GridBagConstraints.WEST;
        gridLayout.add(idField,gbc);

        nomeField = new JTextField(20);
        gbc.gridx = fieldX;
        gbc.gridy = ++fieldY;
        gbc.anchor = GridBagConstraints.WEST;
        gridLayout.add(nomeField,gbc);

        sexoField = new JTextField(10);
        gbc.gridx = fieldX;
        gbc.gridy = ++fieldY;
        gbc.anchor = GridBagConstraints.WEST;
        gridLayout.add(sexoField,gbc);

        rgField = new JTextField(10);
        gbc.gridx = fieldX;
        gbc.gridy = ++fieldY;
        gbc.anchor = GridBagConstraints.WEST;
        gridLayout.add(rgField,gbc);

        cpfField = new JTextField(10);
        gbc.gridx = fieldX;
        gbc.gridy = ++fieldY;
        gbc.anchor = GridBagConstraints.WEST;
        gridLayout.add(cpfField,gbc);

        dtnascField = new JTextField(10);
        gbc.gridx = fieldX;
        gbc.gridy = ++fieldY;
        gbc.anchor = GridBagConstraints.WEST;
        gridLayout.add(dtnascField,gbc);

        dtcadField = new JTextField(10);
        gbc.gridx = fieldX;
        gbc.gridy = ++fieldY;
        gbc.anchor = GridBagConstraints.WEST;
        gridLayout.add(dtcadField,gbc);

        estcivField = new JTextField(10);
        gbc.gridx = fieldX;
        gbc.gridy = ++fieldY;
        gbc.anchor = GridBagConstraints.WEST;
        gridLayout.add(estcivField,gbc);

        panelCenter.add(gridLayout);

        panelPrincipal.add(panelCenter,BorderLayout.CENTER);
    }
    private void Button(){

        panelButton = new JPanel(new FlowLayout());

        btnCad = new JButton("Cadastrar");
        btnCad.addActionListener(this);
        panelButton.add(btnCad);

        panelPrincipal.add(panelButton,BorderLayout.SOUTH);
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        if(btnCad == e.getSource()){

            ControllerPessoa ControlPessoa = new ControllerPessoa();
            Pessoa pessoa = new Pessoa();
            pessoa.setPes_nome(nomeField.getText());
            pessoa.setPes_sexo(sexoField.getText());

            try {
                if(ControlPessoa.cadastroPessoa(pessoa)){
                    JOptionPane.showMessageDialog(null, "Cadastroado com Sucesso");
                    idField.setText(String.valueOf(PessoaDAO.returnID));
                }
            } catch (HeadlessException erro) {
                erro.printStackTrace();
            } catch (ClassNotFoundException erro) {
                JOptionPane.showMessageDialog(null, "ERRO de DriverManager Banco de Dados" +erro.getMessage());
            } catch (SQLException erro) {
                JOptionPane.showMessageDialog(null, "ERRO de Banco de Dados" +erro.getMessage());
            }       
        }
    }

}

---

Class ControllerPessoa:

public class ControllerPessoa {

    public boolean cadastroPessoa(Pessoa pessoa) throws ClassNotFoundException, SQLException{
        PessoaDAO pessoaDAO = new PessoaDAO();  
        String sql = "INSERT INTO farmacia.pessoa1 (nome,sexo) VALUES ('"+ pessoa.getPes_nome() +"','"+ pessoa.getPes_sexo() +"')";

        if(pessoaDAO.insert(sql)){
            return true;
        }       
        return false;
    }       
}

---

Class DAO:

public abstract class DAO {

    public abstract boolean insert (String SQL);
    public abstract boolean update (String SQL);
    public abstract boolean delete (String SQL);
    public abstract boolean select (String SQL);
}

---

Class PessoaDAO:

public class PessoaDAO extends DAO {

    PreparedStatement pstm;
    private Connection con;
    public static int returnID;

    public PessoaDAO() throws ClassNotFoundException, SQLException{
        BD db = new BD();
        con = db.getConn();
    }

    @Override
    public boolean insert(String SQL) {
        try {       
            pstm = con.prepareStatement(SQL,Statement.RETURN_GENERATED_KEYS);   
            pstm.execute();
            int result = pstm.executeUpdate();
            ResultSet rs = pstm.getGeneratedKeys();         
            if (rs.next()){     
                returnID = rs.getInt("id"); // teria outro modo de recuperar o id? sem usa static?
                JOptionPane.showMessageDialog(null, "ID : "+ returnID);
             }
             return result > 0;         

        } catch (Exception e) {
            JOptionPane.showMessageDialog(null, "ERRO : inserir novo pessoa " + e.getMessage());
        }
        return false;
    }

    @Override
    public boolean update(String SQL) {
        // TODO Auto-generated method stub
        return false;
    }

    @Override
    public boolean delete(String SQL) {
        // TODO Auto-generated method stub
        return false;
    }

    @Override
    public boolean select(String SQL) {
        // TODO Auto-generated method stub
        return false;
    }
}

Answer 1

1. Respect naming conventions. Rename the following variables and methods:

   • Center in class TelaPrincipal -> center
   • Button in class TelaPrincipal -> button
   • SQL as a parameter of the methods of DAO and PessoaDAO -> sql
   • getPes_nome in class Pessoa -> getNome
   • setPes_nome in class Pessoa -> setNome
   • getPes_sexo in class Pessoa -> getSexo
   • setPes_sexo in class Pessoa -> setSexo
   • ControlPessoa in the method actionPerformed class TelaPrincipal -> controlPessoa.

2. Responsibility for the SQL statement belongs to the DAO and not to the controller. An SQL statement is a detail of how persistence and data access is accomplished, which is beyond the scope of the controller that is to manage the application flow.

3. Your code suffers from SQL injection problem. With this, I could attack your application when trying to insert a person with the following name:
   x','x'); DROP TABLE farmacia.pessoa1; --
   And then your database is gone.

4. You’re not finishing the Connection, the PreparedStatement and the ResultSet properly. The ideal is to use the syntax Try-with-Resources Java 7 or higher to handle this situation.

5. You are running the insert twice:

pstm.execute();
int result = pstm.executeUpdate();

Before we continue to the main point of your question, we will correct these problems above. For this you will have to change the method insert of PessoaDAO:

public boolean insert(String nome, String sexo) {
    String sql = "INSERT INTO farmacia.pessoa1 (nome, sexo) VALUES (?, ?)";
    BD db = new DB();
    try (Connection con = db.getConn(); PreparedStatement pstm = con.prepareStatement(sql, Statement.RETURN_GENERATED_KEYS)) {
        pstm.setString(1, nome);
        pstm.setString(2, sexo);
        int result = pstm.executeUpdate();
        try (ResultSet rs = pstm.getGeneratedKeys()) {
            if (rs.next()) {
                returnID = rs.getInt("id"); // teria outro modo de recuperar o id? sem usa static?
                JOptionPane.showMessageDialog(null, "ID : "+ returnID);
            }
        }
        return result > 0;

    } catch (Exception e) {
        JOptionPane.showMessageDialog(null, "ERRO : inserir novo pessoa " + e.getMessage());
    }
    return false;
}

And also the builder, who becomes empty:

public PessoaDAO() {
}

And then you can (read: "must") remove the field pstm and con class PessoaDAO.

6. The insert failure is an exceptional situation that should not happen if there is nothing wrong, don’t you think? In this case, rather than return true or false to indicate whether it went right or wrong, the ideal would be to make an exception if it went wrong. And this frees you the return value to return the entered customer code, solving the problem you requested.

7. Do not mix visualization logic with infrastructure logic. In this case, imagine that in the future you want to port the system to a web environment. Ideally you don’t need to change a single comma in your DAOs. However, that JOptionPane will make things not work as expected. However, you use the JOptionPane just to show the exception, the ideal is to cast the exception for it to be treated in a top layer.

Let’s fix the two problems above. First we set a custom exception:

public class DAOException extends Exception {
    public DAOException(String message) {
        super(message);
    }

    public DAOException(String message, Throwable cause) {
        super(message, cause);
    }
}

And then we use the code and solve these two problems:

public int insert(String nome, String sexo) throws DAOException {
    String sql = "INSERT INTO farmacia.pessoa1 (nome, sexo) VALUES (?, ?)";
    BD db = new DB();
    try (Connection con = db.getConn(); PreparedStatement pstm = con.prepareStatement(sql, Statement.RETURN_GENERATED_KEYS)) {
        pstm.setString(1, nome);
        pstm.setString(2, sexo);
        int result = pstm.executeUpdate();
        try (ResultSet rs = pstm.getGeneratedKeys()) {
            if (rs.next()) {
                return rs.getInt("id");
            }
        }
        throw new DAOException("Não foi possível obter o id da pessoa inserida.");
    } catch (ClassNotFoundException e) {
        throw new DAOException("Driver do banco de dados não encontrado.", e);
    } catch (SQLException e) {
        throw new DAOException("Erro no banco de dados ao inserir nova pessoa.", e);
    }
}

And in class PessoaDAO, you can remove the field returnID class, which is what you wanted. :)

Your controller looks like this:

public class ControllerPessoa {
    public int cadastroPessoa(Pessoa pessoa) throws DAOException {
        PessoaDAO pessoaDAO = new PessoaDAO();
        return pessoaDAO.insert(pessoa.getNome(), pessoa.getSexo());
    }       
}

And in your class TelaPrincipal:

        try {
            int returnID = controlPessoa.cadastroPessoa(pessoa);
            idField.setText(String.valueOf(returnID));
            JOptionPane.showMessageDialog(null, "Cadastrado com Sucesso");
        } catch (HeadlessException erro) {
            erro.printStackTrace();
        } catch (DAOException erro) {
            JOptionPane.showMessageDialog(null, erro.getMessage());
        }

8. After making these changes, your super class DAO will not be compatible with the subclass PessoaDAO. What is the best solution? Well, I think the best solution is to simply remove the class DAO. The reason for this is that you never use a reference to the superclass, only references to the subclass. In this case, you’re also not having any kind of advantage in practice by making the subclass adhere to the shape of the superclass. Soon, your superclass is useless and you can remove the extends DAO of the subclass as well as the notes @Override.

9. Your class TelaPrincipal implements the ActionListener button. This is a anti-pattern, because you are delegating the logic of managing the click on the screen button as a whole. This is worse if there are multiple buttons, because instead of you having the managers naturally separate, you would end up putting them all together in one thing only to then separate them using a spaghetti code filled with ifs and switches. Finally, this is a violation of the object-oriented programming model. The solution to this is to delegate the ActionListener correctly.

My suggestion is to do the following:

private void botaoCadastroClicado(ActionEvent e) {
    // Não precisa do `if(btnCad == e.getSource())` para verificar qual foi o botão.
    // O resto do código.
}

And then you add the ActionListener button thus, if you have Java 7:

btnCad.addActionListener(new ActionListener() {
    @Override
    public void actionPerformed(ActionEvent e) {
        botaoCadastroClicado(e);
    }
});

Or in this way much simpler in Java 8:

btnCad.addActionListener(this::botaoCadastroClicado);

10. The fields of your class TelaPrincipal should have the modifier private.

11. I don’t like GridBagLayout. Behold this video to understand why. But to be honest I don’t know if this bug was fixed or not (google gave me conflicting and contradictory information and I’m not in a position to test it now).

Your alternatives in this regard are:

• Let go.
• Use the BoxLayout.
• Use this simple workaround I found at this link:

public class PreferredGridBagLayout extends GridBagLayout {
    @Override
    protected GridBagLayoutInfo getLayoutInfo(Container parent, int sizeflag) {
        return super.getLayoutInfo(parent, PREFERREDSIZE);
    }
}

• Implement your own layout manager.
• Use the BorderLayout and in some parts, maybe you can use Containers or JContainers intermediaries with FlowLayout and GridLayout.
• Using the layout manager null and make absolute positioning with the methods setBounds, setPosition and setSize.
• Use some other workaround that you invent or find.

Anyway, there are several possibilities. But since this is not the focus of the question, I won’t stay here any longer.

12. Consider using a enum for sex, would be much more appropriate than Strings.

NOTE: Oh yes, I’m imagining that the ClassNotFoundException and the SQLException class BD come from the method getConn and not the class builder BD. If it comes from the class builder BD, you will need a try the most in the method insert. I’m also assuming that it’s okay to create multiple instances of this object, since that’s what you did indirectly when creating the controller and DAO whenever the insert button was clicked. If there’s a problem with that, post your class BD to take a look.