patternjavaModerate
Asking a user to take money from a checking account and move it to a savings account
Viewed 0 times
moneyusertakecheckingandaccountsavingsmoveaskingfrom
Problem
I'm aiming to get this program right as I might show it at a job interview. The code below runs fine but I would like to know if there's anything that can be improved in terms of readability, good practices and/or standards. The program asks a user to take money from a checking account and move it to a savings account. This information then gets updated in a MySQL database I set up locally.
Implementation class
```
package com.jdbcbank;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Scanner;
public class Implementation {
// Database access
static final String DB_NAME = "jdbc_example";
static final String HOSTNAME = "127.0.0.1";
static final int DB_PORT = 3306;
static final String DB_URL = "jdbc:mysql://" + HOSTNAME + ":" + DB_PORT
+ "/" + DB_NAME;
// Database credentials
static final String DB_USR = "root";
static final String DB_PASS = "";
// Connection variables
static Connection conn = null;
static Statement statement = null;
static ResultSet result = null;
static PreparedStatement pStatement = null;
public static void main(String[] args) throws SQLException {
// Exercise is done with user id 1
int userId = 1;
// 1. checking, 2. saving
int balance[] = new int[2];
try {
// Connect to database
Implementation.connectToDb();
// Get current balance
BankAccount bankAccount = new BankAccount();
balance = bankAccount.getBalance(userId);
System.out.println("---------------");
System.out.println("Current balance ");
System.out.println("---------------");
System.out.println("Checking Account: " + balance[0] + " USD\n"
+ "Savings Account: " + balance[1] + " USD\n");
checkIfCheckingBalancePositive(balance[0]); // Throw exception if
// balance i
Implementation class
```
package com.jdbcbank;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Scanner;
public class Implementation {
// Database access
static final String DB_NAME = "jdbc_example";
static final String HOSTNAME = "127.0.0.1";
static final int DB_PORT = 3306;
static final String DB_URL = "jdbc:mysql://" + HOSTNAME + ":" + DB_PORT
+ "/" + DB_NAME;
// Database credentials
static final String DB_USR = "root";
static final String DB_PASS = "";
// Connection variables
static Connection conn = null;
static Statement statement = null;
static ResultSet result = null;
static PreparedStatement pStatement = null;
public static void main(String[] args) throws SQLException {
// Exercise is done with user id 1
int userId = 1;
// 1. checking, 2. saving
int balance[] = new int[2];
try {
// Connect to database
Implementation.connectToDb();
// Get current balance
BankAccount bankAccount = new BankAccount();
balance = bankAccount.getBalance(userId);
System.out.println("---------------");
System.out.println("Current balance ");
System.out.println("---------------");
System.out.println("Checking Account: " + balance[0] + " USD\n"
+ "Savings Account: " + balance[1] + " USD\n");
checkIfCheckingBalancePositive(balance[0]); // Throw exception if
// balance i
Solution
Overall it's not bad, but here are a few things that stuck out for me.
-
I'd consider renaming
-
Consider creating and returning an array of
-
It's a little unclear what the effect is of the user entering a number. The prompt says "Enter the amount (in USD) you wish to move your savings account: " Are you moving money from the savings account or to the savings account? Also, the name of the methods
-
In some places you're using the
-
This is pretty bad:
-
This is a nit-pick of mine, but there's no point in initializing your
-
The code doesn't verify that the amount entered by the user is not greater than the current checking account balance. Also note that ideally this check would be performed inside of a transaction along with the actual update to both accounts.
-
I'd consider renaming
getBalance to getBalances, since you're actually returning the balances from multiple bank accounts for that user.balance = bankAccount.getBalance(userId);-
Consider creating and returning an array of
AccountBalance type from the getBalance method instead of an array of int. I don't like the fact that your code just knows that balance[0] is the checking account balance and balance[1] is the savings account balance. In my mind, the AccountBalance type would be a data object, with the following fields: Id, Name, Last 4 digits of account number, and Amount. Well, I've reconsidered this suggestion, because it's a prelude to something quite a bit more complicated than what you need. I'd go with the other answer's suggestion of just having a type that has separate fields for Checking Account Amount and Savings Account Amount. That seems to map more closely with your database, anyway.-
It's a little unclear what the effect is of the user entering a number. The prompt says "Enter the amount (in USD) you wish to move your savings account: " Are you moving money from the savings account or to the savings account? Also, the name of the methods
setAmount and doTransaction are pretty vague as to the purpose. Would something like moveAmountFromCheckingToSavings be more accurate?-
In some places you're using the
BigDecimal datatype for an amount and in others you're using int.-
This is pretty bad:
private static int amount; This is not thread-safe, and I don't see the purpose of doing this instead of just passing the value in as a parameter to the doTransaction method. I don't see the purpose of having two separate methods setAmount and doTransaction; they might as well be one method, without the static variable.-
This is a nit-pick of mine, but there's no point in initializing your
int balance[] variable with new int[2], since you're just going to assign it a totally different value later: balance = bankAccount.getBalance(userId)-
The code doesn't verify that the amount entered by the user is not greater than the current checking account balance. Also note that ideally this check would be performed inside of a transaction along with the actual update to both accounts.
Code Snippets
balance = bankAccount.getBalance(userId);Context
StackExchange Code Review Q#52265, answer score: 13
Revisions (0)
No revisions yet.