java - 有谁知道告诉我这段代码是否可重构？如果是这样怎么办？

Question

    private void fillInvoiceListForName(String name, ArrayList<Invoice> mInvoices) {
    for (SupplierAccount account : listcontracts) {
        if (account.getSupplier() != null)
            if (account.getSupplier().equals(name)) {
                ArrayList<Contract> mContracts = account.getContracts();
                for (Contract contract : mContracts) {
                    mInvoices.addAll(contract.getInvoices());
                }
            }
    }
}

private void fillIncomeListForName(String name, ArrayList<Income> mIncomes) {
    for (SupplierAccount account : listcontracts) {
        if (account.getSupplier() != null)
            if (account.getSupplier().equals(name)) {
                ArrayList<Contract> mContracts = account.getContracts();
                for (Contract contract : mContracts) {
                    mIncomes.addAll(contract.getIncomes());
                }
            }
    }
}


private void fillDocumentListForName(String name, ArrayList<Document> mDocuments) {
    for (SupplierAccount account : listcontracts) {
        if (account.getSupplier() != null)
            if (account.getSupplier().equals(name)) {
                ArrayList<Contract> mContracts = account.getContracts();
                for (Contract contract : mContracts) {
                    mDocuments.addAll(contract.getDocuments());
                }
            }
    }
}

Answer 1

你所有的方法都有共同的迭代。您要做的是抽象迭代方法，同时允许调用者指定要对正在迭代的对象执行的操作。本质上，您希望将内部迭代器（执行迭代）与策略（执行操作）结合起来。

使用策略模式，您可以定义具有共同点的不同策略，然后轻松地用一种替换另一种。在这种情况下，您的所有方法都从合同列表中收集信息并将其添加到列表中，尽管它们收集的信息各不相同。

重构方法

private <E> void fillListForName(String name, List<? super E> listToFill, FillStrategy<E> fillStrategy) {
    if (name == null) {
        throw new IllegalArgumentException("name cannot be null");
    }
    for (SupplierAccount account : listContracts) {
        if (name.equals(account.getSupplier())) {
            List<Contract> contracts = account.getContracts();
            for (Contract contract : contracts) {
                fillStrategy.execute(listToFill, contract);
            }
        }
    }
}

接口和FillStrategy示例实现

interface FillStrategy<T> {
    public void execute(List<? super T> listToFill, Contract contract);
}

class InvoiceFillStrategy implements FillStrategy<Invoice> {
    @Override
    public void execute(List<? super Invoice> listToFill, Contract contract) {
        listToFill.addAll(contract.getInvoices());
    }   
}

调用重构的方法

List<Invoice> invoices = new ArrayList<Invoice>();
InvoiceFillStrategy invoiceStrategy = new InvoiceFillStrategy();

System.out.println("Invoices for myCorp:");
fillListForName("myCorp", invoices, invoiceStrategy);
for (Invoice i : invoices) {
    System.out.println(i);
}

System.out.println("\nInvoices for otherCorp:");
invoices.clear();
fillListForName("otherCorp", invoices, invoiceStrategy);
for (Invoice i : invoices) {
    System.out.println(i);
}

为什么？

这种方法的好处是您可以创建额外的“策略”，而不必修改任何其他涉及的类。例如，您可以创建一个收集超过给定阈值的所有发票的发票：

class ExpensiveInvoiceFillStrategy implements FillStrategy<Invoice> {

    private int minimumAmount;

    public ExpensiveInvoiceFillStrategy(int minimumAmount) {
        this.minimumAmount = minimumAmount;
    }

    @Override
    public void execute(List<? super Invoice> listToFill, Contract contract) {
        for (Invoice invoice : contract.getInvoices()) {
            if (invoice.getAmount() >= minimumAmount) {
                listToFill.add(invoice);
            }
        }
    }
}

只需实现这个类，然后fillListForName用它的实例调用就足够了 - 不需要更改fillListForName或更改Contract！

还有其他方法可以实现迭代器方法和策略——有些甚至会被认为比我在这里所做的“更纯粹”或“更好”。我选择这种方法是因为它使代码与您的代码相似，并且因为我们正在尝试解决特定问题，而不是在 Java 中实现对内部迭代器的一般支持（更聪明的人已经在努力）。请注意，它不是“完美的” :)

Answer 2

考虑使用Guava，它具有强大的操作集合的实用程序。例如，您可以FluentIterable与Predicates 和Functions 结合使用，以提取通用逻辑，并使用它们来转换和过滤您的集合。

下面我将常用元素提取到filterAndTransform允许您传入 a 的方法中，以便您可以从对象Function中收集您喜欢的内容：Contract

private <T> List<T> filterAndTransform(String name, Function<Contract, Iterable<T>> function) {
    return FluentIterable.from(listcontracts)
        .filter(new HasSupplierPredicate())
        .filter(new SupplierNameMatchesPredicate(name))
        .transformAndConcat(new Function<SupplierAccount, Iterable<Contract>>() {
            @Override public Iterable<Contract> apply(final SupplierAccount account) {
                return account.getContracts();
            }
        })
        .transformAndConcat(function)
        .toList();
}

private void fillInvoiceListForName(String name, ArrayList<Invoice> mInvoices) {
    final Iterable<Invoice> invoices = filter(name, 
        new Function<Contract, Iterable<Invoice>>() {
            @Override public Iterable<Invoice> apply(final Contract contract) {
                return contract.getInvoices();
            }
    });
    mInvoices.addAll(invoices);
}

private void fillIncomeListForName(String name, ArrayList<Income> mIncomes) {
    final Iterable<Income> incomes = filter(name, 
        new Function<Contract, Iterable<Income>>() {
            @Override public Iterable<Income> apply(final Contract contract) {
                return contract.getIncomes();
            }
    });
    mIncomes.addAll(incomes);
}

// etc...

Answer 3

定义一个枚举，并将其作为参数传递给方法：

public enum ContractValue {
    INVOICE, INCOME, DOCUMENT;
}

private void fillIncomeListForName(String name, ArrayList<Income> mIncomes, ContractValue contractValue) {
for (SupplierAccount account : listcontracts) {
    if (account.getSupplier() != null)
        if (account.getSupplier().equals(name)) {
            ArrayList<Contract> mContracts = account.getContracts();
            for (Contract contract : mContracts) {
                if (contractValue == ContractValue.INCOME) {
                     mIncomes.addAll(contract.getIncomes()); 
                } else if (contractValue == ContractValue.INVOICE) {
                     mInvoices.addAll(contract.getInvoices());
                } else if (contractValue == ContractValue.DOCUMENT) {
                     mDocuments.addAll(contract.getDocuments());
                }
            }
        }
    }
}