[Java] Make yourself refactored two days ago.

9 minute read

Currently I am picked up by a certain company and undergo training as an engineer. The coin game created during the training was very bad, and I left it as a commandment to myself, which was corrected by the instructor.

Code I wrote

Here is the code I wrote.

public Integer execute() {
    if(this.possessionCoin == 0) {
        return 0;
    } else {
        System.out.println("You have" + this.possessionCoin + "Coin, Start the game? y / n");
        while(true) {
            try {
                int getCoin = 0;
                BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
                String inputChoice = br.readLine();
                System.out.println(inputChoice);
                if(inputChoice.equals("y")) {
                    System.out.println("Please bet Coin 1 ~ "+ this.maxBetCoin);
                    while(true) {
                        try {
                            BufferedReader br2 = new BufferedReader(new InputStreamReader(System.in));
                            String inputStr = br2.readLine();
                            Integer inputBetCoin = Integer.parseInt(inputStr);
                            if(0 <inputBetCoin && inputBetCoin <= this.possessionCoin && inputBetCoin <= this.maxBetCoin) (
                                this.possessionCoin -= inputBetCoin;
                                int sumCardScore = this.getCard();
                                if(this.judgeCard(sumCardScore)) {
                                    getCoin = inputBetCoin * 2;
                                    System.out.println("You Win! Get" + getCoin + "Coin!");
                                    DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame(getCoin, this.deckSetCount);
                                    int winCoinCount = doubleUpChanceGame.execute();
                                    if(winCoinCount == 0) {
                                        return 0;
                                    } else {
                                        System.out.println("You got "+ winCoinCount + "Coin !!");
                                        this.execute();
                                    }
                                } else {
                                    this.possessionCoin = 0;
                                    break;
                                }
                                
                            } else {
                                break;
                            }
                        } catch(IOException | NumberFormatException e) {
                            
                        }
                    }
                } else if(inputChoice.equals("n")) {
                    return this.possessionCoin;
                } else {
                    System.out.println("Please enter y or n.");
                }
                if(getCoin >0) {
                    System.out.println("You got "+ getCoin + "Coin !!");
                } else {
                    System.out.println("You are losing");
                }
                PlayLogs playLog = new PlayLogs();
                playLog.export();
                this.execute();
                
            } catch(IOException | NumberFormatException e) {
                
            }
        }
    }
    
}

You don’t need to be a programmer to understand how bad the code is. It is an amazing 10-tier nest. (Lol) But two days ago, I was calmly writing: Perhaps those who have read this article will press the back button the moment they see this code. By the way, please be aware that the code is not complete in some places because the nesting becomes too deep and you can not do it yourself.

Then, let’s make a time slip in the past and then hang around and fix yourself. By the way, since the process is too long, the method should be separated by nature, but since I want to focus on making the nest shallow, I do not mention method extraction. Please understand that as well.

Point 1 Delete the else using the early return

First from the top if statement If the current coin is 0, 0 is returned, and if not, it is the next process.

before


public Integer execute() {
    if(this.possessionCoin == 0) {
        return 0;
    } else {
        System.out.println("You have" + this.possessionCoin + "Coin, Start the game? y / n");

Modify it as follows.

after


public Integer execute() {
    if(this.possessionCoin == 0) {
        return 0;
    }
    System.out.println("You have" + this.possessionCoin + "Coin, Start the game? y / n");

If it is true according to the result of the if statement, it is returned, so the else under it is not necessary. This alone made the nest shallower. You don’t need to write else if you return after applying to the process like this. Early returns and guard clauses make it easier to read by actively using them.

Point 2 In try, write only the processing that may cause an exception

before


try {
    int getCoin = 0;
    BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
    String inputChoice = br.readLine();
    System.out.println(inputChoice);
    if(inputChoice.equals("y")) {
        System.out.println("Please bet Coin 1 ~ "+ this.maxBetCoin);
        while(true) {
            try {
                BufferedReader br2 = new BufferedReader(new InputStreamReader(System.in));
                String inputStr = br2.readLine();
                Integer inputBetCoin = Integer.parseInt(inputStr);
                if(0 <inputBetCoin && inputBetCoin <= this.possessionCoin && inputBetCoin <= this.maxBetCoin) (
                    this.possessionCoin -= inputBetCoin;int sumCardScore = this.getCard();
                    if(this.judgeCard(sumCardScore)) {
                        getCoin = inputBetCoin * 2;
                        System.out.println("You Win! Get" + getCoin + "Coin!");
                        DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame(getCoin, this.deckSetCount);
                        int winCoinCount = doubleUpChanceGame.execute();
                        if(winCoinCount == 0) {
                            return 0;
                        } else {
                            System.out.println("You got " + winCoinCount + "Coin !!");
                            this.execute();
                        }
                    } else {
                        this.possessionCoin = 0;
                        break;
                    }
                    
                } else {
                    break;
                }
            } catch(IOException | NumberFormatException e) {
                
            }   
        }
    } else if(inputChoice.equals("n")) {
        return this.possessionCoin;
    } else {
        System.out.println("Please enter y or n.");
    }
    if(getCoin > 0) {
        System.out.println("You got " + getCoin + "Coin !!");
    } else {
        System.out.println("You are losing");
    }
    PlayLogs playLog = new PlayLogs();
    playLog.export();
    this.execute();
    
} catch(IOException e) {
    
}      

すさまじいですね(笑)tryの中だけで50行ぐらいあります。ここをtryは例外が起こりえる場所に絞って記述するといった意識で取り組みます。 まず最初のtryの中で例外が起こりえるのは

String inputChoice = br.readLine();

この一文のみです。そのため以下のように修正します。

try {
    inputChoice = br.readLine();
} catch(IOException e) {

}

他の記述は外に出してしまいます。そうすると変数のスコープの関係上エラーが出ますが、外に変数を作ればいいだけなので作ります。 tryする記述を絞ることでこれまたネストを浅くすることができます。

after


try {
    inputChoice = br.readLine();
    System.out.println(inputChoice);
    
} catch(IOException e) {

}     
if(inputChoice.equals("y")) {
    System.out.println("Please bet Coin 1 ~ " + this.maxBetCoin);
    while(true) {

tryの中からif文を出せたことで少しすっきりしました。

次は外に出したif文をすっきりさせていきましょう。

before


// この上にwhile文があります。
if(inputChoice.equals("y")) {
    System.out.println("Please bet Coin 1 ~ " + this.maxBetCoin);
    while(true) {
        try {
            BufferedReader br2 = new BufferedReader(new InputStreamReader(System.in));
            String inputStr = br2.readLine();
            Integer inputBetCoin = Integer.parseInt(inputStr);
            if(0 < inputBetCoin && inputBetCoin <= this.possessionCoin && inputBetCoin <= this.maxBetCoin) {
                this.possessionCoin -= inputBetCoin;
                int sumCardScore = this.getCard();
                if(this.judgeCard(sumCardScore)) {
                    getCoin = inputBetCoin * 2;
                    System.out.println("You Win! Get" + getCoin + "Coin!");
                    DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame(getCoin, this.deckSetCount);
                    int winCoinCount = doubleUpChanceGame.execute();
                    if(winCoinCount == 0) {
                        return 0;
                    } else {
                        System.out.println("You got " + winCoinCount + "Coin !!");
                        this.execute();
                    }
                } else {
                    this.possessionCoin = 0;
                    break;
                }
                
            } else {
                break;
            }
        } catch(IOException e) {
            
        }   
    }
} else if(inputChoice.equals("n")) {
    return this.possessionCoin;
} else {
    System.out.println("Please enter y or n.");
}

そしてさらにこれはelseを削除できるのでelseを削除します。 ついでにelse ifもifに直しましょう。(ここはお好みで)

after


if(inputChoice.equals("y")) {
    break;
} 
if(inputChoice.equals("n")) {
    return this.possessionCoin;
}
System.out.println("Please enter y or n.");

ここまでくるとだいぶ見やすくなりました。

if(inputChoice.equals("y")) {
    break;
} 
if(inputChoice.equals("n")) {
    return this.possessionCoin;
}
System.out.println("Please enter y or n.");
while(true) {
        int getCoin = 0;
        BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
        String inputChoice = "";
    try {
        inputChoice = br.readLine();
        System.out.println(inputChoice);
        
    } catch(IOException e) {

    }     
    if(inputChoice.equals("y")) {
        break;
    } 
    if(inputChoice.equals("n")) {
        return this.possessionCoin;
    }
    System.out.println("Please enter y or n.");
    
    if(getCoin > 0) {
        System.out.println("You got " + getCoin + "Coin !!");
    } else {
        System.out.println("You are losing");
    }
    PlayLogs playLog = new PlayLogs();
    playLog.export();
}
// ここまで修正完了
//--------------------------------------------------------------

while(true) {
    System.out.println("Please bet Coin 1 ~ " + this.maxBetCoin);
    try {
        String inputStr = br.readLine();Integer inputBetCoin = Integer.parseInt(inputStr);
        if(0 < inputBetCoin && inputBetCoin <= this.possessionCoin && inputBetCoin <= this.maxBetCoin) {
            this.possessionCoin -= inputBetCoin;
            int sumCardScore = this.getCard();
            if(this.judgeCard(sumCardScore)) {
                getCoin = inputBetCoin * 2;
                System.out.println("You Win! Get" + getCoin + "Coin!");
                DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame(getCoin, this.deckSetCount);
                int winCoinCount = doubleUpChanceGame.execute();
                if(winCoinCount == 0) {
                    return 0;
                } else {
                    System.out.println("You got " + winCoinCount + "Coin !!");
                    this.execute();
                }
            } else {
                this.possessionCoin = 0;
                break;
            }
            
        } else {
            break;
        }
    } catch(IOException e) {
        
    }   

あとは下のwhile文のtryの中を外に出します。

try {
    String inputStr = br.readLine();
    Integer inputBetCoin = Integer.parseInt(inputStr);
} catch(IOException | NumberFormatException e) {
    System.out.println("You typed incorrect value, please type of correct number");
} 

after


while(true) {
    System.out.println("Please bet Coin 1 ~ " + this.maxBetCoin);
    try {
        String inputStr = br.readLine();
        Integer inputBetCoin = Integer.parseInt(inputStr);
    } catch(IOException | NumberFormatException e) {
        System.out.println("You typed incorrect value, please type of correct number");
    }

    //次はこの下を修正  
    if(0 < inputBetCoin && inputBetCoin <= this.possessionCoin && inputBetCoin <= this.maxBetCoin) {
        this.possessionCoin -= inputBetCoin;
        int sumCardScore = this.getCard();
        if(this.judgeCard(sumCardScore)) {
            getCoin = inputBetCoin * 2;
            System.out.println("You Win! Get" + getCoin + "Coin!");
            DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame(getCoin, this.deckSetCount);
            int winCoinCount = doubleUpChanceGame.execute();
            if(winCoinCount == 0) {
                return 0;
            } else {
                System.out.println("You got " + winCoinCount + "Coin !!");
                this.execute();
            }
        } else {
            this.possessionCoin = 0;
            break;
        }
        
    } else {
        break;
    }
}

ポイント3 はじける条件は先に書く(continueの使用)

if文を修正します。今回の条件は入力値(ベット額)が0より大きくベットできる最大額以下の時、かつ現在の所持コイン以下だった場合に処理を行いたいので、逆に考えてこの条件以外の時はその後の処理ははじいて入力を再度求める処理を書けばよさそうです。ここでcontinueを使います。

if(0 < inputBetCoin && inputBetCoin <= this.possessionCoin && inputBetCoin <= this.maxBetCoin) {
    処理
}

この部分を

if(inputBetCoin <= 0) {
    continue;
}
if(inputBetCoin > this.possessionCoin) {
    continue;
}
if(inputBetCoin > this.maxBetCoin) {
    continue;
}
処理

このように書き換えました。 こうすることで入力値が0以下の時はやり直し、所持コインよりも大きいときはやり直し、ベット可能最大額より大きいときはやり直しといった記述ができました。このほうがシンプルで読みやすいと思います。

ポイント4 複雑な条件の時は処理を小分けにする。

&&でつなげてもよいのですが、人間の目は横より縦に長いほうが読みやすく条件1つずつのほうが前の条件を気にしなくていいので上記のような書き方にしました。


次はif文の中にif文が書いてあるのでここを修正します。

before


if(this.judgeCard(sumCardScore)) {
    getCoin = inputBetCoin * 2;
    System.out.println("You Win! Get" + getCoin + "Coin!");
    DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame(getCoin, this.deckSetCount);
    int winCoinCount = doubleUpChanceGame.execute();
    if(winCoinCount == 0) {
        return 0;
    } else {
        System.out.println("You got " + winCoinCount + "Coin !!");
        this.execute();
    }
} else {
    break;
}

まず中のif文を外に出します。

after


int winCoinCoint = 0;
if(this.judgeCard(sumCardScore)) {
    getCoin = inputBetCoin * 2;
    System.out.println("You Win! Get" + getCoin + "Coin!");
    DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame(getCoin, this.deckSetCount);
    winCoinCount = doubleUpChanceGame.execute();
} else {
    break;
}
if(winCoinCount == 0) {
    return 0;
} else {
    System.out.println("You got " + winCoinCount + "Coin !!");
    break;
}

1つ目のif文がelseになったらbreakするようにすればbeforeの時と同じ動きになります。変数のスコープでエラーが発生するので修正します。

おなじみifでアーリーリターンしているので下のほうのelseを削除します。

if(this.judgeCard(sumCardScore)) {
    getCoin = inputBetCoin * 2;
    System.out.println("You Win! Get" + getCoin + "Coin!");
    DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame(getCoin, this.deckSetCount);
    winCoinCount = doubleUpChanceGame.execute();
} else {
    break;
}
if(winCoinCount == 0) {
    return 0;
}
System.out.println("You got " + winCoinCount + "Coin !!");
break;

ポイント5 条件を逆に考える

上記のif文にはもう一つ直せそうな箇所があります。 それはelseに処理が移った時にbreakしかしていないということです。

before


if(this.judgeCard(sumCardScore)) {
    getCoin = inputBetCoin * 2;
    System.out.println("You Win! Get" + getCoin + "Coin!");DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame(getCoin, this.deckSetCount);
    winCoinCount = doubleUpChanceGame.execute();
} else {
    break;
}

The method called this.judgeCard is a method that returns a boolean. Now when false is returned, you can write this because you can exit the process with break.

after


if(!this.judgeCard(sumCardScore)) {
    break;
}
getFromCardPickCoin += (inputBetCoin * 2);
System.out.println("You Win! Get" + (getFromCardPickCoin) + "Coin!");
DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame((getFromCardPickCoin), this.deckSetCount);
getFromDoubleUpCoin = doubleUpChanceGame.execute();

By doing this, you can break and exit the process when it becomes false, and continue the process otherwise. Now you can delete the else again. However, it may be difficult to understand the judgment of overturning a boolean in !, so it may be better to devise the variable name or use it on a case-by-case basis.

Fixed

Before correction

before


public Integer execute() {
    if(this.possessionCoin == 0) {
        return 0;
    } else {
        System.out.println("You have" + this.possessionCoin + "Coin, Start the game? y / n");
        while(true) {
            try {
                int getCoin = 0;
                BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
                String inputChoice = br.readLine();
                System.out.println(inputChoice);
                if(inputChoice.equals("y")) {
                    System.out.println("Please bet Coin 1 ~ "+ this.maxBetCoin);
                    while(true) {
                        try {
                            BufferedReader br2 = new BufferedReader(new InputStreamReader(System.in));
                            String inputStr = br2.readLine();
                            Integer inputBetCoin = Integer.parseInt(inputStr);
                            if(0 <inputBetCoin && inputBetCoin <= this.possessionCoin && inputBetCoin <= this.maxBetCoin) (
                                this.possessionCoin -= inputBetCoin;
                                int sumCardScore = this.getCard();
                                if(this.judgeCard(sumCardScore)) {
                                    getCoin = inputBetCoin * 2;
                                    System.out.println("You Win! Get" + getCoin + "Coin!");
                                    DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame(getCoin, this.deckSetCount);
                                    int winCoinCount = doubleUpChanceGame.execute();
                                    if(winCoinCount == 0) {
                                        return 0;
                                    } else {
                                        System.out.println("You got "+ winCoinCount + "Coin !!");
                                        this.execute();
                                    }
                                } else {
                                    this.possessionCoin = 0;
                                    break;
                                }
                                
                            } else {
                                break;
                            }
                        } catch(IOException | NumberFormatException e) {
                            
                        }
                    }
                } else if(inputChoice.equals("n")) {
                    return this.possessionCoin;
                } else {
                    System.out.println("Please enter y or n.");
                }
                if(getCoin >0) {
                    System.out.println("You got "+ getCoin + "Coin !!");
                } else {
                    System.out.println("You are losing");
                }
                PlayLogs playLog = new PlayLogs();
                playLog.export();
                this.execute();
                
            } catch(IOException | NumberFormatException e) {
                
            }
        }
    }
    
}

Revised

after


public Integer execute() {
    if(this.possessionCoin == 0) {
        return 0;
    }
    BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
    while(true) {
        System.out.println("You have" + this.possessionCoin + "Coin, Start the game? y / n");
        String inputChoice = "";
        try {
            inputChoice = br.readLine();
        } catch(IOException | NumberFormatException e) {
            System.out.println("You typed incorrect value, please type of correct number");
        }
        if(inputChoice.equals("n")) {
            return this.possessionCoin;
        }
        if(inputChoice.equals("y")) {
            break;
        }
        System.out.println("Please enter y or n.");
    }
    
    int inputBetCoin = 0;
    int getFromCardPickCoin = 0;
    int getFromDoubleUpCoin = 0;
    
    while(true) {
        System.out.println("Please bet Coin 1 ~ "+ this.maxBetCoin);
        try {
            String inputStr = br.readLine();
            inputBetCoin = Integer.parseInt(inputStr);
        } catch(IOException | NumberFormatException e) {System.out.println("You typed incorrect value, please type of correct number");
        } 
        if(inputBetCoin <= 0) {
            continue;
        }
        if(inputBetCoin > this.maxBetCoin) {
            continue;
        }
        if(inputBetCoin > this.possessionCoin) {
            continue;
        }
        this.possessionCoin -= inputBetCoin;
        int sumCardScore = this.getCard();
        if(!this.judgeCard(sumCardScore)) {
            break;
        }
        getFromCardPickCoin += (inputBetCoin * 2);
        System.out.println("You Win! Get" + (getFromCardPickCoin) + "Coin!");
        DoubleUpChanceGame doubleUpChanceGame = new DoubleUpChanceGame((getFromCardPickCoin), this.deckSetCount);
        getFromDoubleUpCoin = doubleUpChanceGame.execute();
        if(getFromDoubleUpCoin == 0) {
            System.out.println("You are losing");
            return this.possessionCoin;
        }
        break;
    }
    if(getFromDoubleUpCoin > 0) {
        System.out.println("You got " + getFromDoubleUpCoin + "Coin !!");
        this.possessionCoin += getFromDoubleUpCoin + inputBetCoin;
    } else {
        System.out.println("You are losing");
    }
    PlayLogs playLog = new PlayLogs();
    playLog.export();
    return this.execute();
}

ネストを3段にまで減らすことができました。行数が多く読みにくいかもしれませんがそれでも修正前よりだいぶ読みやすくなったと思います。 ここからメソッドを切り分ければさらに読みやすくなると思います。 (修正前はアプリが未完成の状態で修正後がアプリの完成版なのでところどころロジックや変数名が変わっていますが、今回の主眼はアプリの作成ではなくネストを浅くすることだったのでそちらについてはご容赦ください。)

最後に

ネストが深くなっているときには絶対に何かしら他の方法があるといった考えを持つことが必要だと感じました。 とりあえず2日前の自分のことはぶっ飛ばしたのでよしとします(笑) もしもっと見やすくするにはこうしたほうが良いといった意見やこういったところにも気を配ったほうが良いなどありましたらぜひコメントいただけると幸いです!

Tags:

Updated: