首页
学习
活动
专区
圈层
工具
发布
社区首页 >问答首页 >要求用户从支票账户取钱并将其转移到储蓄账户。

要求用户从支票账户取钱并将其转移到储蓄账户。
EN

Code Review用户
提问于 2014-06-02 12:47:23
回答 7查看 4.8K关注 0票数 15

我的目标是使这个程序正确,因为我可能会在面试中展示它。下面的代码运行良好,但我想知道在可读性、良好实践和/或标准方面是否有什么可以改进的地方。该程序要求用户从支票账户中提取钱,并将其转移到储蓄账户。然后在本地设置的MySQL数据库中更新此信息。

实现类

代码语言:javascript
复制
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();
        }
}

BankAccount类

代码语言:javascript
复制
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美元

EN

回答 7

Code Review用户

发布于 2014-06-03 13:09:48

除了提到的所有其他优点之外,还有:

  • 此代码大量混合了表示逻辑、业务逻辑和后端数据库实现细节。这是三件不同的事情;它们应该在代码中保持很远的距离。
  • 这段代码上写满了"sql注入攻击“。不要使用字符串来构建SQL查询。使用存储过程或允许将数据作为参数传递的任何其他技术。(现在您使用的是数字,它通常可以安全地抵御SQL攻击,但这仍然是非常糟糕的样式。养成正确做事的习惯。)
  • 你的银行账户的实现与银行如何模拟真实的银行账户完全没有任何相似之处。银行从不使用破坏性更新。如果你在一个账户里存了100美元,而你存了50美元,银行就不会在某个数据库里找到你的余额,把它擦去,换成150美元。相反,他们保留了每笔交易的完整清单,并在清单末尾加上“存入50美元”。然后,可以从该事务列表中计算出任何时间点的余额。(事实上,一些银行使用不支持破坏性更新的特殊硬盘;一旦写入一个字节,它就会永远保留这个值。)
票数 13
EN

Code Review用户

发布于 2014-06-02 14:44:15

代码结构中有太多的静力学。消除对您的应用程序没有有效全局或常量的所有静力学。具体来说,BankAccount.amountBankAccount.getAmount()不能是静态的。

考虑使用范围内的尝试资源(自Java 7起)来获取和关闭连接、语句和结果集。如果使用Java 7不是一个选项:您的close方法有一个复制粘贴问题,您需要检查pStatement,但关闭conn。(“最后”的使用看起来很棒。)

BankAccount.getBalance()有多个问题

  1. 它写入Implementation.result,而不是将结果集用作本地。
  2. 然后循环这些结果,但只返回最后一行。
  3. 您似乎使用int[2]作为类型;请考虑为其创建一个单独的命名类型。

这里有吹毛求疵!

您的错误处理非常粗糙:print("Error: " + e.getMessage())消除了堆栈跟踪和异常类型,这两种类型对于调试都非常有用。要么抛出异常,要么记录整个堆栈。

checkIfAmountPositivecheckIfCheckingBalancePositive有着相似的用途,但方法签名不同。

withdrawFromChecking可以使余额保持负值。考虑将查询扩展到:

代码语言:javascript
复制
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.");
}
票数 10
EN

Code Review用户

发布于 2014-06-02 15:09:54

假设你是面试官--试着从自己的角度去理解他们是如何看待你的代码的,而不知道你对这段代码的了解。

使用合理的名称

public class Implementation {中的实现是不好的开始。您需要解释“这是用于测试我的逻辑是否有效以及用于用户输入和输出的实现”。它不仅向您展示了您的实现类不满足SRP,而且它也没有好的名称。在这里编写(单元)测试可能是首选的。

许多变量(如userIdscanner )也是如此。我需要你的意见,以了解什么是在这些变量-但良好的命名,不需要评论。为什么不和testUserIdtransferAmountScanner一起去呢?请考虑查看您的代码并考虑更合适的变量名称。不过,其中一些当然没问题。

不使用硬编码配置值

代码中有像static final String DB_USR = "root";System.out.println("New balance ");这样的行。假设数据库用户名更改了,或者您想要添加多语言支持,或者只是修复拼写错误。然后,您需要重建您的软件。最好通过使用一个配置文件来避免这种情况,在这个配置文件中,所有这些东西都在一个文件中,这样,如果您想要更改某些内容,并且更加灵活,您就会发现速度更快。

如果是面试,你可以使用硬编码的值,但一定要让面试官知道你这样做是为了简单,而在一个真正的项目中,你会使用配置。

在明显的

时使用类

虽然在您的示例代码中,只使用balance的一个基本类型是可以的,但是考虑创建一个Balance类,或者至少让面试官知道他们会在一个真正的项目中这样做,因为机会很大,因此还需要额外的功能。日志类(Es)也是如此。您甚至可以创建一个事务类。

准备的报表

使用预先准备好的语句,即使您似乎对查询输入有完全的控制,也会显示您了解安全风险,并知道规避这些风险的适当工具和技术。

使用javas类

您使用简单数组int balance[] = new int[2];。除非您必须处理性能关键问题,否则永远不要使用它们。最好使用ArrayList或其他数据结构(或者--在本例中--创建像上面建议的新类)

独立于

平台的

使用硬编码的行分隔符(如在"Savings Account: " + balance[1] + " USD\n");中)可能不错,但您更愿意使用配置文件中定义的分隔符。在这种情况下,您还应该考虑使用java line.separator属性来获取应用程序正在运行的系统上使用的"\r\n""\r""\n"或任何行分隔符。当您在运行与您的操作系统不同的计算机上显示时,您不希望您的新行被吃掉。

票数 10
EN
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接:

https://codereview.stackexchange.com/questions/52265

复制
相关文章

相似问题

领券
问题归档专栏文章快讯文章归档关键词归档开发者手册归档开发者手册 Section 归档