Using PostgreSQL effectively with a banking app

Question

I'm kind of newbie for these things but I did something and I want to know how I can do this effectively.

import java.sql.Connection;
import java.sql.Statement;
import java.sql.ResultSet;
import java.sql.DriverManager;
import java.sql.SQLException;
import java.util.Random;

public class BankingApp
{
    public static void main(String[] args)
    {
        Registeration myreg = new Registeration("vivian","stuff","[email protected]");
        myreg.setDatabaseInfo("JavaAppFirst","root","134679");
        myreg.beRegister();
    }
}

interface SQLConnection
{
    public boolean setConnection() throws Exception; // implement the sql connection
    // public void closeConnection(Connection c,Statement stmt) throws SQLException; // close connection
}

class Registeration implements SQLConnection
{
    private int id;
    private String usr,pwd,email;
    private Connection connect;
    private Statement state;
    private String dbName,dbUsername,dbPassword;

    private void tempRegister() throws Exception
    {
        connect.setAutoCommit(false);
        state = connect.createStatement();
        String sqlQuery = "INSERT INTO users (id,uname,upass,uemail) VALUES ('"+this.id+"','"+this.usr+"','"+this.pwd+"','"+this.email+"');";
        state.executeUpdate(sqlQuery);
        state.close();
        connect.commit();
        connect.close();
    }

    public void beRegister()
    {
        try
            {
                if( setConnection() != true )
                    {
                        System.err.println("[+] Connection Error");
                        return;
                    }
                tempRegister();
                System.out.println("[+] Registeration Successfully");
            }catch(Exception e)
            {
                System.err.println(e.getClass().getName()+": "+e.getMessage());
                System.exit(0);
            }
    }

    public boolean setConnection() throws Exception
    {
        connect = null;
        state = null;
        Class.forName("org.postgresql.Driver");
        connect = DriverManager.getConnection("jdbc:postgresql://localhost:5432/"+this.dbName,this.dbUsername,this.dbPassword);
        if( connect == null ) return false;
        return true;
    }

    public void setDatabaseInfo(String a,String b,String c)
    {
        this.dbName = a;
        this.dbUsername = b;
        this.dbPassword = c;
    }

    public Registeration()
    {
        //Random random = new Random();
        id = 0; // random.nextInt(2000);
        usr = null;
        pwd = null;
        email = null;
    }
    public Registeration(String usr,String pwd,String email)
    {
        Random random = new Random();
        this.id = random.nextInt(1000);
        this.usr = usr;
        this.pwd = pwd;
        this.email = email;
    }
}

In my view this code is horrible. How can I develop this code?

Sleiman Jneidi · Accepted Answer · 2014-12-05 21:04:10Z

As you mentioned in the question, this code is really horrible.

Have you considered using an ORM, Hibernate for example?
You shouldn't append strings to do build queries, use Prepared Statements instead. This is the mistake number 3 in the list of top 25 most dangerous mistakes.
You shouldn't hard code database information in you business objects.
```
 Class.forName("org.postgresql.Driver"); // shouldn't be here
```
you can store these information in a configuration file where you can read these values from.
Use a logging framework instead of System.out.println and System.err.println.

The application shouldn't terminate if you if the registration fails.

 catch(Exception e)
    {
        System.err.println(e.getClass().getName()+": "+e.getMessage());
        System.exit(0); // horrible
    }

This will kill the process. You can re-throw the exception instead

catch(Exception e)
    {
        System.err.println(e.getClass().getName()+": "+e.getMessage());
        throw e;
    }

Catching Exception in general is usually a bad idea, you should be more specific so you don't end up handling exception that you didn't want to handle.
```
catch(SQLException e)
    {
        log.error(e.getClass().getName()+": "+e.getMessage(),e);
        throw e;
    }
```

thank you for your knowledge. Your Number 3 can you be more spesific about configuration file. Is there a any example you can show me ? I understood the others. — Vivian Maya, Dec 6 '14 at 0:19
@VivianMaya no problem. There are plenty for ways for dealing with config files in Java. The simplest one is to have a properties file. crunchify.com/… — Sleiman Jneidi, Dec 6 '14 at 0:25

Malachi · Answer 2 · 2014-12-05 20:06:08Z

this looks really ugly from a readability/formatting point of view.

    public void beRegister()
    {
        try
            {
                if( setConnection() != true )
                    {
                        System.err.println("[+] Connection Error");
                        return;
                    }
                tempRegister();
                System.out.println("[+] Registeration Successfully");
            }catch(Exception e)
            {
                System.err.println(e.getClass().getName()+": "+e.getMessage());
                System.exit(0);
            }
    }

You should use consistent Egyptian (Java) bracing for this along with making sure that you leave some space around the Braces and not around the parenthesis.

Key words also like their space from parenthesis.

Look how much cleaner this little piece of code looks now.

public void beRegister() {
    try {
        if (setConnection() != true) {
            System.err.println("[+] Connection Error");
            return;
        }
        tempRegister();
        System.out.println("[+] Registeration Successfully");
    } catch (Exception e) {
        System.err.println(e.getClass().getName()+": "+e.getMessage());
        System.exit(0);
    }
}

It is about my emacs configuration,I'm really more confortable with this style but I really thank you for your opinion. Actually I'm looking for way to give me more control on the code. On my question the code is so disorganized. — Vivian Maya, Dec 5 '14 at 20:11

ferada · Answer 3 · 2014-12-05 20:21:05Z

up vote 2 down vote

I'll just post a few things; use Prepared Statements, otherwise you'll be vulnerable to SQL injection attacks, respectively it's a good idea to always use them instead of building your queries as a string like it's done in tempRegister.

setConnection() != true is the same as setConnection() == false, which is the same as !setConnection() which is the best way to write that. For boolean values it's redundant to check with == or !=, so best avoid that.

answered Dec 5 '14 at 20:21

ferada
3,404727

I have seen PreparedStatement, I had trouble about String combining and I used it once but I didn't know about it is avoiding SQL Injections attacks. Thank you for your information, and for second part of your post it is really nice info. Thank you for information , I'm greatful... – Vivian Maya Dec 5 '14 at 20:25

2

In your example it is technically fine, but once you have users and they enter e.g. their own email address or password, the problem comes up immediately. So better be safe from the start. – ferada Dec 5 '14 at 20:26

add a comment |

Jamal · Answer 4 · 2014-12-20 00:33:16Z

From a database standpoint, you should not be randomly generating user IDs:

public Registeration(String usr,String pwd,String email)
{
    Random random = new Random();
    this.id = random.nextInt(1000);
    this.usr = usr;
    this.pwd = pwd;
    this.email = email;
}

If the id column is unique (and it should be if you don't have any other unique constraints on this table), you're going to end up with a unique constraint violation eventually. Instead, you should be relying on the features of your database to do this for you. PostgreSQL has sequences, use them.

I'm also a very concerned that you're inserting the passwords as plain text. Anyone who has access to your database will be able to read this information. PostgreSQL has a few handy functions you can use to encrypt your passwords (see this).

One last thing: don't abbreviate short variable names. You aren't really saving all that much by shortening user to usr or password to pwd. Abbreviating just makes it more difficult to remember in 6 months (now how exactly did I abbreviate this variable again?). It will be even worse for the next guy who has to maintain this code.

thank you for your time, what would you suggest for creating unique id ? — Vivian Maya, Dec 16 '14 at 5:14

asked	9 months ago
viewed	108 times
active	8 months ago

current community

your communities

more stack exchange communities

Using PostgreSQL effectively with a banking app

4 Answers 4

Your Answer

Not the answer you're looking for? Browse other questions tagged java beginner oop postgresql or ask your own question.

Hot Network Questions

current community

your communities

more stack exchange communities

Using PostgreSQL effectively with a banking app

4 Answers 4

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged java beginner oop postgresql or ask your own question.

Related

Hot Network Questions