【问题标题】:Should I return a collection when the reference to the collection is not changed?当对集合的引用未更改时,我应该返回集合吗?
【发布时间】:2014-04-02 13:40:16
【问题描述】:

我得到了一个接受如下集合的方法

 public IList<CountryDto> ApplyDefaults(IList<CountryDto> dtos)
        {
            //Iterates the collection
                //Validates the items in collection
                //If items are invalid
                //Removes items e.g dtos.Remove(currentCountryDto)

            return dtos;//Do I need to do this?
        }

我的问题是,对集合的引用没有改变,我应该从方法中再次返回集合吗?

  1. 对于:通过返回集合,我在签名中明确表示并且用户知道集合中的项目可能与原始来源不同。避免歧义。
  2. 反对:由于验证不会更改集合的引用,因此在技术上返回它没有意义。

在这种情况下,最好的方法是什么?
注意:我不确定这个问题是否基于意见。我想我可能在设计方面遗漏了一些东西。

【问题讨论】:

  • 您是否关心通知用户是否应用了验证/默认值?
  • 好点。否。此服务应用对用户透明的默认值。
  • 您是否关心创建与 LINQ-to-Objects 兼容的 fluent-API?例如:ApplyDefaults(myList).Where(dto =&gt; dto.Name == "Canada").Select(dto =&gt; dto.Currency).ToList()?编辑:或者您只是希望用户像“一劳永逸”一样就地过滤/验证列表:var myCountries = DtoService.GetCountries(); ApplyDefaults(myCountries); DoSomething(myCountries);
  • ApplyDefaults 中发生的事情要复杂得多。我正在调用其他服务来执行注入类的验证。所以我不认为 fluent-api 是我的选择。

标签: c# java c++ oop


【解决方案1】:

在每种编程语言中,您自己的代码/库与核心库的方法的一致性具有很高的价值。因此,检查 Collections.sort() 或 Collection.swap() 和 Collections.shuffle() 是如何定义的,如果您打算修改它,我建议不要返回输入参数。此外,您的方法应该以这样的方式命名,很明显输入参数被修改了。否则你的方法会被认为有副作用。

返回一个值通常表明它是一个反映工作的新实例,由方法执行或在builders 的情况下用于方法链接。

【讨论】:

    【解决方案2】:

    鉴于您的 cmets/要求:

    1. 如果应用默认值则不需要报告。
    2. ApplyDefaults 很复杂,会调用其他服务,并且不打算生成流畅的 API
    3. ApplyDefaults 是一个“黑匣子”;验证逻辑被注入,因此调用代码不知道/不关心验证

    我认为基于这些,这个方法绝对应该返回对传入列表的引用,即使没有应用验证。首先,除非 API 明确地围绕方法链接构建(您表示不想要),否则返回 List&lt;T&gt; 类型通常表示正在创建 new 列表。其次,如果没有创建了一个新列表,用户可能会发现自己正在以他们没有预料到的方式修改列表。

    考虑:

    IList<CountryDto> originalCountries = Service.GetCountries();
    IList<CountryDto> validatedCountries = ApplyDefaults(originalCountries);
    validatedCountries.Add(mySpecialCountry);
    
    OutputOriginalCountries(originalCountries);
    OutputValidatedCountries(validatedCountries);
    

    这段代码不是很特别,而且是相当常见的模式。如果ApplyDefaults 返回对同一originalCountries 集合的引用,则mySpecialCountry 也将添加到originalCountries。这将违反最小惊讶原则。

    如果此行为根据项目是否经过验证/过滤而发生变化,则会加剧这种情况。由于验证逻辑是调用者不知道或不关心的行为黑盒,API 使用者可能取决于它是否返回相同的引用。他们要么必须自己做参考检查(例如,if (myValidatedCountries == myInputCountries)),要么每次都复制一份。无论如何,这将成为程序员在使用 API 时必须处理的另一个奇怪行为。

    我认为该方法应该:

    A) 总是返回一个过滤掉项目的复制列表 (public IList&lt;CountryDto&gt; ApplyDefaults(IEnumerable&lt;CountryDto&gt; dtos))

    B) 就地修改传入列表 (public void ApplyDefaults(IList&lt;CountryDto&gt; dtos))

    对于选项 A,根据列表的大小,这可能会导致每次创建复制列表的不必要工作,即使不执行过滤也是如此。但是,验证/过滤逻辑可能更简单。您也许可以使用 LINQ 查询很好地应用过滤。此外,从列表中删除项通常代价高昂,因为它必须重建内部数组。因此,建立一个新列表实际上可能会更快。您甚至可以将这里的签名简化为IEnumerable&lt;CountryDto&gt;;这允许更广泛的使用,并且非常很明显您正在创建一个新集合。

    对于选项 B,如果不需要验证,则不做任何工作,并且该方法本质上是“免费的”(不重建数组、不复制、不更改引用)。但如果有重要的验证,删除方面可能会很昂贵。由于您不是方法链接,因此此版本应该具有void 返回类型,因为对于开发人员来说,这是就地修改列表更加明显。这遵循其他常见的方法,如List&lt;T&gt;.Sort。此外,如果用户想要拥有单独的 originalCountriesvalidatedCountries,他们可以随时制作副本:

    var validatedCountries = originalCountries.ToList();
    ApplyDefaults(validatedCountries);
    

    最终,您选择哪一种可能取决于性能。如果验证/删除既便宜又罕见,那么就地修改列表可能是最好的。如果您希望对列表进行大量更改,那么每次生成一个新副本可能会更快。

    无论如何,我建议您也更清楚地命名该方法。例如:

    public IList<CountryDto> GetValidCountries(IEnumerable<CountryDto> dtos)
    
    public void RemoveInvalidCountries(IList<CountryDto> dtos)
    

    当然,根据您的实际代码上下文,命名可能会有所不同(我怀疑ApplyDefaults 是一个通用/继承的方法名称,而不是特定于CountryDto

    【讨论】:

      【解决方案3】:

      我宁愿返回boolean(或enum,在一个详细的情况下:集合完好无损已更改无法验证等)

      // true if the collection is changed, false otherwise
      public Boolean ApplyDefaults(IList<CountryDto> dtos) {
        Boolean result = false;
        //Iterates the collection
        //Validates the items in collection
        //If items are invalid:
        //  Removes items e.g dtos.Remove(currentCountryDto)
        //  result = true;
        ...
        return result; 
      }
      
      ...
      
      if (ApplyDefaults(myData)) {
        // Collection is changed, do some extra stuff
      }
      

      【讨论】:

        【解决方案4】:

        首先:您不能更改通过参数发送的集合的引用,因为默认情况下您会获得它的副本。您需要使用 ref 关键字才能进行更改。

        其次:如果你的方法有返回类型,那么它必须返回一个对象。您的方法不是称为GetNewCollectionWithAppliedDefaults,而是ApplyDefaults,这意味着将修改集合。您应该返回布尔值 true/false 以通知用户更改已完成,或者始终返回参数的集合(以允许调用嵌套方法)。

        另外,为什么您认为返回集合没有意义?我会说没有反对它的论据。把问题转过来:“为什么我不返回集合,它会损害我的代码”?

        【讨论】:

        • 是的。我应该措辞正确。我的意思是我没有创建新实例或克隆。
        【解决方案5】:

        从技术上讲,我想说两者之间没有太大区别。

        但是,正如您所指出的,一个常用的约定是函数应该只返回它创建的对象。基本上,这意味着一个返回对象的函数正在生成一个对象,而一个不返回任何内容的函数正在修改作为参数传递的对象。

        同样,这只是一个约定,在 C# 社区中并未广泛使用,但在 python 社区中,例如,它是。

        有些人会返回布尔值(或错误代码),而不是作为错误指示符(如旧的 dos 命令行)。我不喜欢这种方法,并且更喜欢引发我以后可以处理的异常。

        最后,在我看来,最好的方法是返回一个值,该值指示函数是否完成了更改,并最终返回一个值,指示完成了多少更改。它可以是布尔值,也可以是插入/删除元素的数量...

        在任何情况下,如果不是在所有代码中,至少在单个项目中,尽量与您选择的方法保持一致。有时,您别无选择,只能遵守队友使用的惯例。

        【讨论】:

          【解决方案6】:

          (我的回答基于 Java 观点;C++ 和 C# 程序员可能有不同的看法。)我认为最好返回集合。您返回的集合与给出的集合是同一集合这一事实只是一个实现细节,在未来的代码版本中,您可能想要更改它。返回的集合可能与传入的集合不同的文档。

          另一方面,如果您想锁定此方法修改集合的设计,请以这种方式记录它并且不要返回集合。我不喜欢这样做,但在某些情况下我可以看到优势。

          【讨论】:

            【解决方案7】:

            在你的情况下,我会留下 void ApplyDefaults 明确说明它在做什么。 此外,在集合本身中使用 ApplyDefaults 可能是个好主意。子类 IList 或 List 或其他任何东西,然后你会这样调用:

            myCollection.ApplyDefaults();
            

            这很明显。

            【讨论】:

              猜你喜欢
              • 2014-08-31
              • 1970-01-01
              • 1970-01-01
              • 1970-01-01
              • 2011-03-09
              • 1970-01-01
              • 1970-01-01
              • 2016-04-14
              • 2010-12-30
              相关资源
              最近更新 更多