是否在此代码中键入检查代码气味？ [关闭]

Question

我有一个界面“IPartyCountService”，它计算客户数量和供应商数量。

实现类“PartyCountService”使用类型检查来检查该方是客户还是供应商。

实现类PartyCountService使用类型检查是否给出代码味道？

任何反馈，评论和批评都表示赞赏。

public interface IPartyCountService
{
    int GetTotalNumberOfCustomers();
    int GetTotalNumberOfSuppliers();
}

internal class PartyCountService:IPartyCountService
{
    IPartyRepository _partyRepository;

    public PartyCountService(IPartyRepository partyRepository)
    {
        _partyRepository = partyRepository;
    }

    public int GetTotalNumberOfCustomers()
    {
        var counter = 0;
        foreach(var customer in _partyRepository.GetAll())
        {
            if (customer is Customer) counter++;
        }
        return counter;
    }

    public int GetTotalNumberOfSuppliers()
    {
        var counter = 0;
        foreach (var customer in _partyRepository.GetAll())
        {
            if (customer is Supplier) counter++;
        }
        return counter;
    }
}

public interface IPartyRepository
{
    IList<IParty> GetAll();
}
internal class PartyRepository:IPartyRepository
{
    public IList<IParty> GetAll()
    {
        // put together all parties, including customers and suppliers
        return allParties;
    }
}
internal class Customer:IParty{}
internal class Supplier:IParty{}
public interface IParty{}

Answer 1

我会使用.OfType <>扩展方法。

return _partyRepository.GetAll().OfType<Customer>().Count();

编辑：如下面的SP所述，这使得一些更清洁的代码，但不一定修复气味。

Answer 2

也许IParty应该有一种方法来询问您在计算“客户”时感兴趣的任何内容？

Answer 3

对我来说感觉不对，但是你可以做出一点改变。

return _partyRepository.GetAll().Count(p => p is Customer);

Answer 4

我认为你不应该隐藏公共API中的Customer和Supplier。但是，建议用ICustomer和ISupplier接口替换它们。基于接口设计在组件的最前端（即其公共API）将帮助您很自然地实现更好的设计。

关于类型检查和计数：我没有看到明确的动态类型检查是坏事。但是，通过使用接口，它们在语义上变得更加自然。另外，为了节省明确的foreach和一对{ }，我不认为LINQ类似的语句应该在任何地方引入。

Answer 5

客户和供应商是否存储在partyRepository中的单个异构集合中？似乎简单地挂在一组客户和一个单独的供应商集合上要容易得多;然后它可以暴露像GetCustomers（），GetSuppliers（）或Get <T>（）这样的函数。

使用这种方法，您仍然可以实现GetAll（） - 将2个集合“联合”在一起非常容易。比过滤异构集合更容易（可能性能更好）。

Answer 6

它有点臭，但不是我见过的（或自己完成的）最臭的东西。几点想法：

• 有没有理由将供应商和客户存放在同一个系列中？如果您发现您的代码始终有所区别，请重构您存储它们的方式。
• 存储一系列接口但随后铸造成混凝土类型会产生异味。每当我看到那种类型的演员表时，我立即认为抽象泄漏或需要第二次看的静态结构。
• 如果将它们组合起来确实有意义，LINQ可以使它更直接。

像这样：

return _partyRepository.GetAll().OfType<Supplier>().Count();

Answer 7

首先，我认为这是一个选择问题。您必须知道每种可能的替代方案所涉及的权衡：

1. 你的。
2. 在基类上创建一个字段，使您可以知道哪种类型的聚会。

替代方案（1）具有更简洁和对OO更友好的方法的优点。 （但是，请记住，当您查看对象的具体类型时，您正在打破多态性。）

替代方案（2）的优点是，首先，您不依赖元数据来了解对象类型。其次，is调用比检查变量类型更昂贵。因此，请确保您能够满足性能要求。

编辑：现在我看一下它，你抽象到一个接口类型，然后把它转换回它的具体类型。这是为什么？如果使用集合的类知道不同的类型，它应该直接访问存储在那里的类型。