java - 清理带有过多条件的代码

Question

我从前段时间发现了这段丑陋的代码:

@FXML
private void buttSellAction(ActionEvent event){
    InfoTip infoTip = new InfoTip();
    if(comboPizza.getValue() != null){
        if(comboPizzaSize.getValue() != null){
            PizzaData selectedPizza = getPizzaData(comboPizza.getValue());
            PizzaSizeData selectedPizzaSize = getPizzaSizeData(comboPizzaSize.getValue());
            Date date = new Date();
            Timestamp timestamp = new Timestamp(date.getTime());
            if( selectedPizza != null ){                    
                if(groupDelivery.getSelectedToggle().equals(radioNo)){ // sale without delivery
                    Alert alert = new Alert(AlertType.CONFIRMATION);
                    alert.setTitle("Confirm");
                    alert.setHeaderText("Total cost: " + String.format("%.2f", selectedPizza.getPrice() + selectedPizzaSize.getPrice()));
                    alert.setContentText("Proceed with sale?");

                    Optional<ButtonType> result = alert.showAndWait();
                    if (result.get() == ButtonType.OK){
                        insertSale(timestamp, currentUser.getLogin(), selectedPizza.getID(), 
                                   selectedPizzaSize.getSize(), false, selectedPizza.getPrice() + selectedPizzaSize.getPrice());

                        infoTip.getInfoTip().setId("greenInfoTip");
                        infoTip.showTip((Button)event.getSource(), "   Saved   ");
                    }
                }else{ //Sale with delivery
                    String adress = textFAdress.getText();
                    String clientName = textFClientName.getText();
                    String telephone = textFTelephone.getText();
                    String deliveryCost = textFCost.getText();

                    boolean isAdressOK = ((adress.length() < 51) && (adress.isEmpty() == false))? true: false;
                    boolean isClientNameOK = (clientName.length() < 36)? true: false;
                    boolean isTelephoneOK = ((telephone.length() < 21) && (telephone.isEmpty() == false))? true: false;
                    boolean isCostOK;
                    try{ Double.valueOf(deliveryCost); isCostOK = true; }
                    catch(NumberFormatException exception){ isCostOK = false; }

                    if(isAdressOK == true){
                        if(isClientNameOK == true){
                            if(isTelephoneOK == true){
                                if(isCostOK == true){
                                    double totalCost = selectedPizza.getPrice() + selectedPizzaSize.getPrice() + Double.valueOf(deliveryCost);
                                    //everything is okey
                                    Alert alert = new Alert(AlertType.CONFIRMATION);
                                    alert.setTitle("Confirm");
                                    alert.setHeaderText("Total cost: " + totalCost);
                                    alert.setContentText("Proceed with sale?");

                                    Optional<ButtonType> result = alert.showAndWait();
                                    if (result.get() == ButtonType.OK){
                                        int id = insertSale(timestamp, currentUser.getLogin(), selectedPizza.getID(), 
                                                selectedPizzaSize.getSize(), true, selectedPizza.getPrice() + selectedPizzaSize.getPrice());

                                        insertDelivery(id, adress, clientName, telephone, Double.valueOf(deliveryCost));

                                        infoTip.getInfoTip().setId("greenInfoTip");
                                        infoTip.showTip((Button)event.getSource(), "   Saved   ");
                                    } else {
                                        // ... user chose CANCEL or closed the dialog
                                    }
                                }else{ //cost not ok
                                    infoTip.showTip(textFCost, "keep right format e.g. 4.35");
                                }
                            }else{ //telephone not ok
                                infoTip.showTip(textFTelephone, "max 20 characters, not empty");
                            }
                        }else{ //client name not ok
                            infoTip.showTip(textFClientName, "max 35 characters");
                        }
                    }else{ //adress not ok
                        infoTip.showTip(textFAdress, "max 50 characters, not empty");
                    }
                }
            }else{ //couldnt found selected pizza in pizzaList(which should not be possible)
                ExceptionDialog exceptionDialog = new ExceptionDialog("Error when searching for selected pizza", new Exception());
                exceptionDialog.showAndWait();
            }
        }else{ //pizza size not choosen
            infoTip.showTip(comboPizzaSize, "select pizza size");
        }
    }else{ //pizza not choosen
        infoTip.showTip(comboPizza, "select pizza");
    }
}

我现在知道它有几个主要缺陷:

• 方法做的太多而且时间太长，
• 它有太多的条件语句，所以很容易迷路，
• 不必要的注释会降低代码的可读性。
• 重复代码，
• 可能是混合级别的复杂性。
• 测试它会很糟糕。
• 还有别的吗？？

我如何重构它以使其简洁明了？

Answer 1

我会对另一个答案采取稍微不同的方法。我会将验证与方法中的其他逻辑分开。这意味着您无需阅读大量 if 语句即可了解该方法的核心逻辑，如果您想更改验证，您只需在一处更新一个语句。例如，对于第一部分，添加一个私有(private)方法:

private PizzaSizeData getAndVerifySelectedPizza() {
    if (comboPizza.getValue() == null) {
        infoTip.showTip(comboPizza, "select pizza");
        return null;
    }

    if (comboPizzaSize.getValue() == null) {
        infoTip.showTip(comboPizzaSize, "select pizza size");
        return null;
    }

    PizzaData selectedPizza = getPizzaData(comboPizza.getValue());
    if (selectedPizza == null) {
        ExceptionDialog exceptionDialog = new ExceptionDialog("Error when searching for selected pizza", new Exception());
        exceptionDialog.showAndWait();
        return null;
    }

    return getPizzaSizeData(comboPizzaSize.getValue());
}

您可以返回可选值而不是 null，但这说明了机制。

然后调用新方法:

private void buttSellAction(ActionEvent event){
    InfoTip infoTip = new InfoTip();

    PizzaSizeData selectedPizzaSize = getAndVerifySelectedPizza();
    if (selectedPizzaSize == null) {
        return;
    }

    // Carry on with the method....

将验证放在带有早期返回语句的方法的开头是一种常见的模式，因此多次返回不会混淆任何人，并且它们允许单独编写每个验证规则。