首页
学习
活动
专区
圈层
工具
发布
社区首页 >问答首页 >IEnumerable of IEnumerable of IEnumerable的不良决策

IEnumerable of IEnumerable of IEnumerable的不良决策
EN

Code Review用户
提问于 2018-05-13 10:50:43
回答 1查看 142关注 0票数 -6

我希望改进类的设计,使我的可读性和性能问题,随着代码的增长。

代码语言:javascript
复制
public class ProductCategory
{
    public string Name { get; }
    public string SalesAmountInUSD { get; }
    public IEnumerable<CurrencyWeight> weightOfEachCurrenciesForProductCategory { get; }
}

public class CurrencyWeight
{
    public string Currency{ get; }
    public double Weight { get; }
    public IEnumerable<RetailerWeight> weightOfEachRetailerInEachCurrencyForAProduct { get; }
}

public class RetailerWeight
{
    public string Retailer { get; }
    public double Weight { get; }   
}

我遇到的第一个问题是可读性,因为不太清楚IEnumerable代表什么。

CurrencyWeight应该是每种货币在总销售额中所占的比例(以美元为单位,因为用同一种货币操作所有货币并尽可能晚地申请兑换货币是比较容易的)。RetailerWeight是指零售商对产品类别的每种货币的销售收入所占的比例。

ProductCategory是从数据库构建的,下面是一个Dto类:

代码语言:javascript
复制
public class ProductDto
{
    public string ProductName{ get; set; }
    public double SaleAmount { get; set; }
    public string Currency { get; set; }
    public double SaleAmountUSD { get; set; }
    public string ProductCategory { get; set; }
    public string Retailer { get; set; }
}

我的第二个问题是使用,因为我将得到一个包含IEnumerable<ProductCategory>IEnumerable<CurrencyWeight>,其中包含IEnumerable<RetailerWeight>,如果我需要在ProductCategory.SalesAmountInUSD上应用一个变更率以及CurrencyWeightRetailerWeight,那么我将有一个三重foreach循环,它的性能可能很差。

一开始,我试图创建字典来解开所有的东西:

代码语言:javascript
复制
public class AggregationKey
{
    public string ProductCategoryName { get; }
    public string Currency { get; set; }
}

public class CurrencyWeightV2
{
    public string Currency{ get; }
    public double Weight { get; }  
}

Dictionnary<AggregationKey, IEnumerable<CurrencyWeightV2>> currencyWeightRepository;

以同样的方式:

代码语言:javascript
复制
public class SubAggregationKey
{
    public string ProductCategoryName { get; }
    public string Currency { get; }
    public string Retailer { get; }
    public AggregationKey getAggregationKey => new AggregationKey(ProductCategoryName, Currency);
}

public class SubContractorWeightV2
{
    public string Retailer{ get; }
    public double Weight { get; }  
}

Dictionnary<SubAggregationKey, IEnumerable<SubContractorWeightV2>> subContractorWeightRepository;

然而,我意识到应用变化率所需的循环数以及CurrencyWeightRetailerWeightProductCategory.SalesAmountInUSD上(例如,我在上面使用的例子)并没有改变,而没有提到按ProductDto分组以填充这些字典的成本。我没有解决我的问题,我只是取代了它,不是吗?

我也不太清楚subContractorWeightRepository代表的是什么。虽然我希望解除IEnumerable<CurrencyWeight>IEnumerable<RetailerWeight>的耦合,但它们本身并没有什么意义,不是吗?

谢谢你抽出时间阅读和回答,如果我忘记了一个重要的信息,请告诉我。

(我没有包含任何构造函数来保持示例代码的小,但它们都只是传递参数来初始化属性,正如您所期望的那样。这里没什么特别的)。

EN

回答 1

Code Review用户

回答已采纳

发布于 2018-05-16 12:53:23

命名

代码语言:javascript
复制
public class CurrencyWeight
{
    public IEnumerable<RetailerWeight> weightOfEachRetailerInEachCurrencyForAProduct { get; }
}

这里使用的命名方法是不正确的。此属性不应指产品。

关系是两个实体之间的关系。但你指的是三个实体:

weightOfEachRetailer InEach 货币 ForA 产品

产品与这种关系没有任何关系。CurrencyWeight是否有ProductCategoryRetailerWeightCurrencyWeight之间的关系并不重要。这违反了封装原则,您正在添加(命名)不必要的引用。

此外,这里也不需要引用货币。这个属性是CurrencyWeight类的一部分,这已经意味着它是“在货币重量中”。

一个更简单的例子是:

代码语言:javascript
复制
public class Employee
{
    public string NameOfEmployee;
}

NameOfEmployee是多余的。Name就足够了,因为它已经是Employee类的一个属性。

这是蓝精灵命名公约的一个例子:

当几乎每个类都有相同的前缀时。也就是说,当用户单击按钮时,SmurfAccountView将一个SmurfAccountDTO传递给SmurfAccountController。SmurfID用于获取一个SmurfOrderHistory,该SmurfOrderHistory在转发到SmurfHistoryReviewView或SmurfHistoryReportingView之前传递给SmurfHistoryMatch。如果发生SmurfErrorEvent,SmurfErrorLogger会将其记录到${app}/smurf/log/smurf/smurflg.log

换句话说,以下名称具有足够的描述性:

代码语言:javascript
复制
public class CurrencyWeight
{
    public IEnumerable<RetailerWeight> RetailerWeights { get; }
}

您试图以属性名称提供的其余信息应从类之间的关系中明显看出。

迭代集合

我的第二个问题是使用,因为我最终会得到一个包含IEnumerable的IEnumerable,其中包含IEnumerable。

这本身并不是一个有问题的情况。如果您有意加载这个实体图,那么这没有什么问题。

例如,如果我想显示一个城市、他们的省和他们的国家的列表,我将有一个IEnumerable<Country>,其中每个Country都有一个IEnumerable<Province>,而每个Province都有一个IEnumerably<City>

您并不总是需要加载所有这些数据,例如,如果您只对一个国家列表感兴趣,那么您所需要的只是一个IEnumerable<Country>,其中每个Country都有一个为null的IEnumerable<Province>属性。

例如,如果我需要在CurrencyWeight和RetailerWeight上应用一个变化率,

如果您需要在许多对象中设置一个值,那么显然需要在内存中设置所需的对象。这是不可避免的。

我将有一个三前程循环,这可能是非常糟糕的表现。

三重风险并不一定是坏的,当他们得到保证的时候。有时候,你可以简化它们。返回农村-省-城市实例:

代码语言:javascript
复制
foreach(var country in myCountryList)
    foreach(var province in country.Provinces)
        foreach(var city in province.Cities)
        {
            city.Name = "Foo";
        }

这可以简化为:

代码语言:javascript
复制
foreach(var city in myCountryList.SelectMany(c => c.Provinces).SelectMany(p => p.Cities))
{
     city.Name = "Foo";
}

但是,如果您需要在每个级别上设置一个属性,那么您就无法避免三层预测:

代码语言:javascript
复制
foreach(var country in myCountryList)
{
    country.Name = "Foo";

    foreach(var province in country.Provinces)
    {
        province.Name = "Bar";

        foreach(var city in province.Cities)
        {
            city.Name = "Baz";
        }
    }
}

两个考虑因素:

  • 避免嵌套的预见主要是一个可读性的论点,而不是一个性能的论点。无论如何,SelectMany内部仍然迭代所有集合。这是不可避免的,因为您不能在不迭代集合的情况下访问集合中的所有项。
  • 迭代多个实体级别并设置大量值的方法表明您违反了SRP。这很可能被分解成不同的方法,以保持固体友好。
票数 1
EN
页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持
原文链接:

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

复制
相关文章

相似问题

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