我的目标是使这个程序正确,因为我可能会在面试中展示它。下面的代码运行良好,但我想知道在可读性、良好实践和/或标准方面是否有什么可以改进的地方。该程序要求用户从支票账户中提取钱,并将其转移到储蓄账户。然后在本地设置的MySQL数据库中更新此信息。
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 is negative
System.out
.print("Enter the amount (in USD) you wish to move your savings account: ");
// Set amount and do transaction
Scanner scanner = new Scanner(System.in);
int amount = checkIfAmountPositive(scanner.nextInt()); // Throw exception if amount is negative
scanner.close();
bankAccount.setAmount(amount);
bankAccount.doTransaction();
// Get new balance
balance = bankAccount.getBalance(userId);
System.out.println();
System.out.println("---------------");
System.out.println("New balance ");
System.out.println("---------------");
System.out.println("Checking Account: " + balance[0] + " USD\n"
+ "Savings Account: " + balance[1] + " USD\n");
System.out.println("Thank you.");
} catch (Exception e) {
System.out.println("Error: " + e.getMessage());
} finally {
// Close all connections
close(conn, statement, pStatement, result);
}
}
public static void checkIfCheckingBalancePositive(int balance) {
if (balance < 0)
throw new IllegalArgumentException(
"Your checking balance is negative, you can't withdraw.");
}
public static int checkIfAmountPositive(int amount) {
if (amount < 0) {
throw new IllegalArgumentException("Amount must be positive");
}
return amount;
}
public static void connectToDb() {
try {
// Load driver
Class.forName("com.mysql.jdbc.Driver");
// Set connection
conn = DriverManager.getConnection(DB_URL, Implementation.DB_USR,
DB_PASS);
conn.setAutoCommit(false);
statement = conn.createStatement();
} catch (ClassNotFoundException e) {
System.out.println("Error: " + e.getMessage());
} catch (SQLException e) {
System.out.println("Error: " + e.getMessage());
}
}
public static void close(Connection conn, Statement statement,
PreparedStatement pStatement, ResultSet result) {
if (conn != null)
try {
conn.close();
} catch (SQLException e) {
System.out.println("Error: " + e.getMessage());
}
if (statement != null)
try {
statement.close();
} catch (SQLException e) {
System.out.println("Error: " + e.getMessage());
}
if (pStatement != null)
try {
conn.close();
} catch (SQLException e) {
e.printStackTrace();
}
if (result != null)
try {
statement.close();
} catch (SQLException e) {
e.printStackTrace();
}
}package com.jdbcbank;
import java.sql.*;
import java.math.*;
public class BankAccount {
private static int amount;
public void doTransaction() throws SQLException {
try {
// Withdraw from checking and deposit into saving
withdrawFromChecking(Implementation.statement, new BigDecimal(amount), 1);
depositIntoSaving(Implementation.statement, new BigDecimal(amount), 1);
// Execute batch and commit if no issues are found
Implementation.statement.executeBatch();
Implementation.conn.commit();
// Print message
System.out.println("Successful transaction!");
} catch (Exception e) {
System.out.println("Error: " + e.getMessage());
Implementation.conn.rollback();
}
}
public int[] getBalance(int id) throws SQLException {
// Array to be returned
int balance[] = new int[2];
// Get results
String query = "SELECT * from bank_account WHERE id = " + id;
Implementation.result = Implementation.statement.executeQuery(query);
while (Implementation.result.next()) {
// Retrieve by column name
balance[0] = Implementation.result.getInt("checking_balance");
balance[1] = Implementation.result.getInt("saving_balance");
}
return balance;
}
public static int getAmount() {
return amount;
}
public void setAmount(int amount) {
BankAccount.amount = amount;
}
public static void withdrawFromChecking(Statement statement,
BigDecimal amount, int id) throws SQLException {
statement
.addBatch("UPDATE bank_account SET checking_balance = checking_balance - "
+ amount + " where id = " + id);
}
public static void depositIntoSaving(Statement statement,
BigDecimal amount, int id) throws SQLException {
statement
.addBatch("UPDATE bank_account SET saving_balance = saving_balance + "
+ amount + " where id = " + id);
}
}-经常余额-支票账户: 380美元储蓄账户: 1120美元-你想转移到你的储蓄账户: 20笔交易成功-新的余额-新的余额-支票账户: 360美元储蓄账户: 1140美元
发布于 2014-06-03 13:09:48
除了提到的所有其他优点之外,还有:
发布于 2014-06-02 14:44:15
代码结构中有太多的静力学。消除对您的应用程序没有有效全局或常量的所有静力学。具体来说,BankAccount.amount和BankAccount.getAmount()不能是静态的。
考虑使用范围内的尝试资源(自Java 7起)来获取和关闭连接、语句和结果集。如果使用Java 7不是一个选项:您的close方法有一个复制粘贴问题,您需要检查pStatement,但关闭conn。(“最后”的使用看起来很棒。)
BankAccount.getBalance()有多个问题Implementation.result,而不是将结果集用作本地。int[2]作为类型;请考虑为其创建一个单独的命名类型。您的错误处理非常粗糙:print("Error: " + e.getMessage())消除了堆栈跟踪和异常类型,这两种类型对于调试都非常有用。要么抛出异常,要么记录整个堆栈。
checkIfAmountPositive和checkIfCheckingBalancePositive有着相似的用途,但方法签名不同。
withdrawFromChecking可以使余额保持负值。考虑将查询扩展到:
UPDATE bank_account
SET checking_balance = checking_balance - ?
WHERE id = ?
AND checking_balance >= ?
int updatedRows = statement.executeUpdate();
if ( updatedRows < 1 ) {
throw new IllegalArgumentException("Balance too low or no such account.");
}发布于 2014-06-02 15:09:54
假设你是面试官--试着从自己的角度去理解他们是如何看待你的代码的,而不知道你对这段代码的了解。
在public class Implementation {中的实现是不好的开始。您需要解释“这是用于测试我的逻辑是否有效以及用于用户输入和输出的实现”。它不仅向您展示了您的实现类不满足SRP,而且它也没有好的名称。在这里编写(单元)测试可能是首选的。
许多变量(如userId或scanner )也是如此。我需要你的意见,以了解什么是在这些变量-但良好的命名,不需要评论。为什么不和testUserId和transferAmountScanner一起去呢?请考虑查看您的代码并考虑更合适的变量名称。不过,其中一些当然没问题。
代码中有像static final String DB_USR = "root";和System.out.println("New balance ");这样的行。假设数据库用户名更改了,或者您想要添加多语言支持,或者只是修复拼写错误。然后,您需要重建您的软件。最好通过使用一个配置文件来避免这种情况,在这个配置文件中,所有这些东西都在一个文件中,这样,如果您想要更改某些内容,并且更加灵活,您就会发现速度更快。
如果是面试,你可以使用硬编码的值,但一定要让面试官知道你这样做是为了简单,而在一个真正的项目中,你会使用配置。
时使用类
虽然在您的示例代码中,只使用balance的一个基本类型是可以的,但是考虑创建一个Balance类,或者至少让面试官知道他们会在一个真正的项目中这样做,因为机会很大,因此还需要额外的功能。日志类(Es)也是如此。您甚至可以创建一个事务类。
使用预先准备好的语句,即使您似乎对查询输入有完全的控制,也会显示您了解安全风险,并知道规避这些风险的适当工具和技术。
您使用简单数组int balance[] = new int[2];。除非您必须处理性能关键问题,否则永远不要使用它们。最好使用ArrayList或其他数据结构(或者--在本例中--创建像上面建议的新类)
独立于
使用硬编码的行分隔符(如在"Savings Account: " + balance[1] + " USD\n");中)可能不错,但您更愿意使用配置文件中定义的分隔符。在这种情况下,您还应该考虑使用java line.separator属性来获取应用程序正在运行的系统上使用的"\r\n"、"\r"、"\n"或任何行分隔符。当您在运行与您的操作系统不同的计算机上显示时,您不希望您的新行被吃掉。
https://codereview.stackexchange.com/questions/52265
复制相似问题