使用循环和集合管道进行重构

循环是处理集合的经典方式,但随着编程语言中一等函数的广泛采用,集合管道 成为了一种有吸引力的替代方案。在本文中,我将通过一系列简单的示例,探讨将循环重构为集合管道的过程。

2015 年 7 月 14 日



编程中的一项常见任务是处理对象列表。大多数程序员自然地使用循环来完成这项任务,因为循环是我们学习的第一批基本控制结构之一。但循环并不是表示列表处理的唯一方式,近年来,越来越多的人开始使用另一种方法,我称之为集合管道。这种风格通常被认为是函数式编程的一部分,但我曾在 Smalltalk 中大量使用它。随着面向对象语言支持 lambda 和使一等函数更容易编程的库,集合管道成为了一种有吸引力的选择。

将简单的循环重构为管道

我将从一个简单的循环示例开始,并展示将循环重构为集合管道的基本方法。

假设我们有一个作者列表,每个作者都有以下数据结构。

class Author...

  public string Name { get; set; }
  public string TwitterHandle { get; set;}
  public string Company { get; set;}

此示例使用 C#

以下是循环。

class Author...

  static public IEnumerable<String> TwitterHandles(IEnumerable<Author> authors, string company) {
    var result = new List<String> ();
    foreach (Author a in authors) {
      if (a.Company == company) {
        var handle = a.TwitterHandle;
        if (handle != null)
          result.Add(handle);
      }
    }
    return result;
  }

将循环重构为集合管道的第一步是将循环集合应用于提取变量[1]

class Author...

  static public IEnumerable<String> TwitterHandles(IEnumerable<Author> authors, string company) {
    var result = new List<String> ();
    var loopStart = authors;
    foreach (Author a in loopStart) {
      if (a.Company == company) {
        var handle = a.TwitterHandle;
        if (handle != null)
          result.Add(handle);
      }
    }
    return result;
  }

此变量为我提供了管道操作的起点。目前还没有一个好的名称,所以我将使用一个有意义的名称,并期望稍后重命名它。

然后我开始查看循环中的行为片段。我看到的第一件事是一个条件检查,我可以使用过滤操作将其移动到管道中。

class Author...

  static public IEnumerable<String> TwitterHandles(IEnumerable<Author> authors, string company) {
    var result = new List<String> ();
    var loopStart = authors
      .Where(a => a.Company == company);
    foreach (Author a in loopStart) {
      if (a.Company == company) {
        var handle = a.TwitterHandle;
        if (handle != null)
          result.Add(handle);
      }
    }
    return result;
  }

我看到循环的下一部分操作的是 Twitter 句柄,而不是作者,因此我可以使用映射操作 [2]

class Author...

  static public IEnumerable<String> TwitterHandles(IEnumerable<Author> authors, string company) {
    var result = new List<String> ();
    var loopStart = authors
      .Where(a => a.Company == company)
      .Select(a => a.TwitterHandle);
    foreach (string handle in loopStart) {
      var handle = a.TwitterHandle;
      if (handle != null)
        result.Add(handle);
     }
    return result;
  }

循环中的下一部分是另一个条件,我也可以将其移动到过滤操作中。

class Author...

  static public IEnumerable<String> TwitterHandles(IEnumerable<Author> authors, string company) {
    var result = new List<String> ();
    var loopStart = authors
      .Where(a => a.Company == company)
      .Select(a => a.TwitterHandle)
      .Where (h => h != null);
    foreach (string handle in loopStart) {
      if (handle != null)
        result.Add(handle);
    }
    return result;
  }

现在循环所做的只是将循环集合中的所有内容添加到结果集合中,因此我可以删除循环,直接返回管道结果。

class Author...

  static public IEnumerable<String> TwitterHandles(IEnumerable<Author> authors, string company) {
    var result = new List<String> ();
    return authors
      .Where(a => a.Company == company)
      .Select(a => a.TwitterHandle)
      .Where (h => h != null);
    foreach (string handle in loopStart) {
      result.Add(handle);
    }
    return result;
  }

以下是代码的最终状态

class Author...

  static public IEnumerable<String> TwitterHandles(IEnumerable<Author> authors, string company) {
    return authors
      .Where(a => a.Company == company)
      .Select(a => a.TwitterHandle)
      .Where (h => h != null);
  }

我喜欢集合管道的一点是,我可以看到列表元素通过管道时的逻辑流。对我来说,它非常接近我如何定义循环的结果:“获取作者,选择那些有公司的作者,并获取他们的 Twitter 句柄,删除任何空句柄”。

此外,这种代码风格即使在使用不同语法和不同管道运算符名称的不同语言中也很熟悉。 [3]

Java

  public List<String> twitterHandles(List<Author> authors, String company) {
    return authors.stream()
            .filter(a -> a.getCompany().equals(company))
            .map(a -> a.getTwitterHandle())
            .filter(h -> null != h)
            .collect(toList());
  }

Ruby

  def twitter_handles authors, company
    authors
      .select {|a| company == a.company}
      .map    {|a| a.twitter_handle}
      .reject {|h| h.nil?}
  end

虽然这与其他示例匹配,但我将用 compact 替换最终的 reject

Clojure

  (defn twitter-handles [authors company]
    (->> authors
         (filter #(= company (:company %)))
         (map :twitter-handle)
         (remove nil?)))

F#

  let twitterHandles (authors : seq<Author>, company : string) = 
       authors
           |> Seq.filter(fun a -> a.Company = company)
           |> Seq.map(fun a -> a.TwitterHandle)
           |> Seq.choose (fun h -> h)

同样,如果我不关心与其他示例的结构匹配,我会将映射和选择合并为一个步骤。

我发现,一旦我习惯了用管道思维,即使在不熟悉的语言中,我也能很快地应用它们。由于基本方法相同,因此从即使是不熟悉的语法和函数名称中进行转换也相对容易。

在管道内进行重构,以及重构为推导式

一旦你将一些行为表达为管道,你就可以通过重新排序管道中的步骤来进行潜在的重构。其中一个操作是,如果你有一个映射操作后跟一个过滤操作,你通常可以将过滤操作移到映射操作之前,如下所示。

class Author...

  static public IEnumerable<String> TwitterHandles(IEnumerable<Author> authors, string company) {
    return authors
      .Where(a => a.Company == company)
      .Where (a => a.TwitterHandle != null)
      .Select(a => a.TwitterHandle);
  }

当你有两个相邻的过滤操作时,你可以使用连接符将它们合并。 [4]

class Author...

  static public IEnumerable<String> TwitterHandles(IEnumerable<Author> authors, string company) {
    return authors
      .Where(a => a.Company == company && a.TwitterHandle != null)
      .Select(a => a.TwitterHandle);
  }

一旦我有了这种形式的 C# 集合管道,即简单的过滤和映射,我就可以用 Linq 表达式替换它。

class Author...

  static public IEnumerable<String> TwitterHandles(IEnumerable<Author> authors, string company) {
    return from a in authors where a.Company == company && a.TwitterHandle != null select a.TwitterHandle;
  }

我认为 Linq 表达式是列表推导式的一种形式,类似地,你也可以在任何支持列表推导式的语言中执行类似的操作。你喜欢列表推导式形式还是管道形式(我更喜欢管道)是一个品味问题。总的来说,管道功能更强大,因为你无法将所有管道重构为推导式。

嵌套循环 - 图书阅读者

作为第二个示例,我将重构一个简单的双重嵌套循环。假设我有一个在线系统,允许读者阅读书籍。我有一个数据服务,可以告诉我每个读者在特定日期阅读了哪些书籍。此服务返回一个哈希表,其键是读者的标识符,值是书籍标识符的集合。

interface DataService…

  Map<String, Collection<String>> getBooksReadOn(Date date);

对于此示例,我将切换到 Java,因为我厌倦了首字母大写的函数名。

以下是循环。

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  for (Map.Entry<String, Collection<String>> e : data.entrySet()) {
    for (String bookId : books) {
      if (e.getValue().contains(bookId) && readers.contains(e.getKey())) {
         result.add(e.getKey());
      }
    }
  }
  return result;
}

我将使用我的第一步,即将循环集合应用于提取变量

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  final Set<Map.Entry<String, Collection<String>>> entries = data.entrySet();
  for (Map.Entry<String, Collection<String>> e : entries) {
    for (String bookId : books) {
      if (e.getValue().contains(bookId) && readers.contains(e.getKey())) {
         result.add(e.getKey());
      }
    }
  }
  return result;
}

这种操作让我非常高兴,因为IntelliJ 的自动重构功能可以让我免于输入这种复杂的类型表达式。

一旦将初始集合放入变量中,我就可以处理循环行为的元素。所有工作都在条件语句内部进行,所以我将从条件语句中的第二个子句开始,并将它的逻辑移动到过滤操作中。

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  final Set<Map.Entry<String, Collection<String>>> entries = data.entrySet().stream()
          .filter(e -> readers.contains(e.getKey()))
          .collect(Collectors.toSet());
  for (Map.Entry<String, Collection<String>> e : entries) {
    for (String bookId : books) {
      if (e.getValue().contains(bookId) && readers.contains(e.getKey()))) {
        result.add(e.getKey());
      }
    }
  }
  return result;
}

使用 Java 的流库,管道必须以一个终端操作(例如收集器)结束。

另一个子句更难移动,因为它引用了内部循环变量。此子句正在测试映射条目值是否包含任何也出现在方法参数中的书籍列表中的书籍。我可以使用集合交集来实现这一点。Java 核心类不包含集合交集方法。我可以使用 Java 的常见集合导向插件(例如GuavaApache Commons)来解决这个问题。由于这是一个简单的教学示例,我将编写一个粗略的实现。

class Utils...

  public static <T> Set<T> intersection (Collection<T> a, Collection<T> b) {
    Set<T> result = new HashSet<T>(a);
    result.retainAll(b);
    return result;
  }

这在这里有效,但对于任何大型项目,我都会使用一个通用库。

现在我可以将子句从循环中移动到管道中。

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  final Set<Map.Entry<String, Collection<String>>> entries = data.entrySet().stream()
          .filter(e -> readers.contains(e.getKey()))
          .filter(e -> !Utils.intersection(e.getValue(), books).isEmpty())
          .collect(Collectors.toSet());
  for (Map.Entry<String, Collection<String>> e : entries) {
    for (String bookId : books) {
      if (e.getValue().contains(bookId) ) {
        result.add(e.getKey());
      }
    }
  }

  return result;
}

现在循环所做的只是返回映射条目的键,因此我可以通过在管道中添加映射操作来删除循环的剩余部分。

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  result = data.entrySet().stream()
          .filter(e -> readers.contains(e.getKey()))
          .filter(e -> !Utils.intersection(e.getValue(), books).isEmpty())
          .map(e -> e.getKey())
          .collect(Collectors.toSet());
  for (Map.Entry<String, Collection<String>> e : entries) {
    for (String bookId : books) {
      result.add(e.getKey());
    }
  }

  return result;
}

然后我可以使用内联临时变量操作 result

public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
  Set<String> result = new HashSet<>();
  Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
  return data.entrySet().stream()
          .filter(e -> readers.contains(e.getKey()))
          .filter(e -> !Utils.intersection(e.getValue(), books).isEmpty())
          .map(e -> e.getKey())
          .collect(Collectors.toSet());
  return result;
 }

查看交集的使用方式,我发现它相当复杂,我必须在阅读时弄清楚它的作用 - 这意味着我应该提取它。 [5]

class Utils...

  public static <T> boolean hasIntersection(Collection<T> a, Collection<T> b) {
    return !intersection(a,b).isEmpty();
  }

class Service...

  public Set<String> getReadersOfBooks(Collection<String> readers, Collection<String> books, Date date) {
    Map<String, Collection<String>> data = dataService.getBooksReadOn(date);
    return data.entrySet().stream()
            .filter(e -> readers.contains(e.getKey()))
            .filter(e -> Utils.hasIntersection(e.getValue(), books))
            .map(e -> e.getKey())
            .collect(Collectors.toSet());
   }

当需要执行类似操作时,面向对象方法处于劣势。在静态实用程序方法和对象上的普通方法之间切换很麻烦。在某些语言中,我可以让它看起来像流类上的一个方法,但在 Java 中我没有这个选项。

尽管存在这个问题,但我仍然发现管道版本更容易理解。我可以将过滤操作合并为一个连接符,但我通常发现将每个过滤操作视为一个单独的元素更容易理解。

设备产品

下一个示例使用一些简单的标准来标记特定区域的首选设备。为了理解它的作用,我需要解释领域模型。我们有一个组织,在不同区域提供设备。当你请求一些设备时,你可能会获得你想要的设备,但通常你会得到替代品,它几乎可以满足你的需求,但可能不太好。举个不太恰当的例子:你在波士顿,想要一台吹雪机,但如果商店里没有,你可能会得到一把铲雪铲作为替代。但是,如果你在迈阿密,他们甚至不提供吹雪机,所以你只能得到一把铲雪铲。数据模型通过三个类来捕获这一点。

图 1:每个产品实例都代表了在某个区域提供特定类型设备以满足特定设备需求的事实。

因此,我们可能会看到以下数据。

products: ['snow-blower', 'snow-shovel']
regions: ['boston', 'miami']
offerings:
  - {region: 'boston', supported: 'snow-blower', supplied: 'snow-blower'}
  - {region: 'boston', supported: 'snow-blower', supplied: 'snow-shovel'}
  - {region: 'miami',  supported: 'snow-blower', supplied: 'snow-shovel'}

我们要查看的代码是一些将其中一些产品标记为首选产品的代码,这意味着该产品是某个区域中某个设备的首选产品。

class Service...

  var checkedRegions = new HashSet<Region>();
  foreach (Offering o1 in equipment.AllOfferings()) {
    Region r = o1.Region;
    if (checkedRegions.Contains(r)) {
      continue;
    }

    Offering possPref = null;
    foreach(var o2 in equipment.AllOfferings(r)) {
      if (o2.isPreferred) {
        possPref = o2;
        break;
      }
      else {
        if (o2.isMatch || possPref == null) {
          possPref = o2;
        }
      }
    }
    possPref.isPreferred = true;
    checkedRegions.Add(r);
  }

此示例使用 C#,因为我是一个反复无常的人。

我的猜测是,这个循环所做的工作有一些容易理解的逻辑,通过重构它,我希望能够揭示这种逻辑。

我的第一步是从外部循环开始,将初始循环变量应用于提取变量

class Service...

  var loopStart = equipment.AllOfferings();
  var checkedRegions = new HashSet<Region>();
  foreach (Offering o1 in loopStart) {
    Region r = o1.Region;
    if (checkedRegions.Contains(r)) {
      continue;
    }

    Offering possPref = null;
    foreach(var o2 in equipment.AllOfferings(r)) {
      if (o2.isPreferred) {
        possPref = o2;
        break;
      }
      else {
        if (o2.isMatch || possPref == null) {
          possPref = o2;
        }
      }
    }
    possPref.isPreferred = true;
    checkedRegions.Add(r);
  }

然后我查看循环的第一部分。它有一个控制变量 checkedRegions,循环使用它来跟踪已经处理过的区域,以避免多次处理同一个区域。这本身就是一个代码异味,但也表明循环变量 o1 只是通往区域 r 的一个垫脚石。我通过在编辑器中突出显示 o1 来确认这一点。一旦我知道这一点,我就知道可以使用映射 来简化它。

class Service...

  var loopStart = equipment.AllOfferings()
    .Select(o => o.Region);
  var checkedRegions = new HashSet<Region>();
  foreach (Region r in loopStart) {
    Region r = o1.Region;
    if (checkedRegions.Contains(r)) {
      continue;
    }

    Offering possPref = null;
    foreach(var o2 in equipment.AllOfferings(r)) {
      if (o2.isPreferred) {
        possPref = o2;
        break;
      }
      else {
        if (o2.isMatch || possPref == null) {
          possPref = o2;
        }
      }
    }
    possPref.isPreferred = true;
    checkedRegions.Add(r);
  }

现在我可以谈谈 checkedRegions 控制变量。循环使用它来避免多次处理同一个区域。目前还不清楚检查区域是否幂等,如果是,我可能会完全避免这种检查(并进行测量以查看对性能是否有显著影响)。由于我不确定,我决定保留这种逻辑,尤其是因为使用管道避免重复项非常简单

class Service...

  var loopStart = equipment.AllOfferings()
    .Select(o => o.Region)
    .Distinct();
  var checkedRegions = new HashSet<Region>();
  foreach (Region r in loopStart) {
    if (checkedRegions.Contains(r)) {
      continue;
    }

    Offering possPref = null;
    foreach(var o2 in equipment.AllOfferings(r)) {
      if (o2.isPreferred) {
        possPref = o2;
        break;
      }
      else {
        if (o2.isMatch || possPref == null) {
          possPref = o2;
        }
      }
    }
    possPref.isPreferred = true;
    checkedRegions.Add(r);
  }

下一步是确定 possPref 变量。我认为将其作为单独的方法处理会更容易,因此应用提取方法

class Service...

  var loopStart = equipment.AllOfferings()
    .Select(o => o.Region)
    .Distinct();
  foreach (Region r in loopStart) {
    var possPref = possiblePreference(equipment, r);
    possPref.isPreferred = true;
  }

  static Offering possiblePreference(Equipment equipment, Region region) {
    Offering possPref = null;
    foreach (var o2 in equipment.AllOfferings(region)) {
      if (o2.isPreferred) {
        possPref = o2;
        break;
      }
      else {
        if (o2.isMatch || possPref == null) {
          possPref = o2;
        }
      }
    }
    return possPref;
  }

我将循环集合提取到一个变量中。

class Service...

  static Offering possiblePreference(Equipment equipment, Region region) {
    Offering possPref = null;
    var allOfferings = equipment.AllOfferings(region);
    foreach (var o2 in allOfferings) {
      if (o2.isPreferred) {
        possPref = o2;
        break;
      }
      else {
        if (o2.isMatch || possPref == null) {
          possPref = o2;
        }
      }
    }
    return possPref;
  }

由于循环现在位于自己的函数中,我可以使用 return 而不是 break。

class Service...

  static Offering possiblePreference(Equipment equipment, Region region) {
    Offering possPref = null;
    var allOfferings = equipment.AllOfferings(region);
    foreach (var o2 in allOfferings) {
      if (o2.isPreferred) {
        return o2;
        break;
      }
      else {
        if (o2.isMatch || possPref == null) {
          possPref = o2;
        }
      }
    }
    return possPref;
  }

第一个条件正在寻找第一个满足条件的产品(如果有)。这是检测操作的工作(在 C# 中称为 First)。 [6]

class Service...

  static Offering possiblePreference(Equipment equipment, Region region) {
    Offering possPref = null;
    var allOfferings = equipment.AllOfferings(region);
    possPref = allOfferings.FirstOrDefault(o => o.isPreferred);
    if (null != possPref) return possPref;
    foreach (var o2 in allOfferings) {
      if (o2.isPreferred) {
        return o2;
      }
        else {
        if (o2.isMatch || possPref == null) {
          possPref = o2;
        }
      }
    }
    return possPref;
  }

最后一个条件比较棘手。它将 possPref 设置为列表中的第一个产品,但如果任何产品通过了 isMatch 测试,它将覆盖该值。但循环并没有在 isMatch 通过时中断,因此任何后续的 isMatch 产品将覆盖该匹配项。因此,为了复制这种行为,我需要使用 LastOrDefault[7]

class Service...

  static Offering possiblePreference(Equipment equipment, Region region) {
    Offering possPref = null;
    var allOfferings = equipment.AllOfferings(region);
    possPref = allOfferings.FirstOrDefault(o => o.isPreferred);
    if (null != possPref) return possPref;
    possPref = allOfferings.LastOrDefault(o => o.isMatch);
    if (null != possPref) return possPref;
    foreach (var o2 in allOfferings) {
       if (o2.isMatch || possPref == null) {
          possPref = o2;
      }
    }
    return possPref;
  }

循环中剩下的最后一部分只是返回第一个项目。

class Service...

  static Offering possiblePreference(Equipment equipment, Region region) {
    Offering possPref = null;
    var allOfferings = equipment.AllOfferings(region);
    possPref = allOfferings.FirstOrDefault(o => o.isPreferred);
    if (null != possPref) return possPref;
    possPref = allOfferings.LastOrDefault(o => o.isMatch);
    if (null != possPref) return possPref;
    return allOfferings.First();
    foreach (var o2 in allOfferings) {
      if (possPref == null) {
        possPref = o2;
      }
    }
    return possPref;
  }

我个人的习惯是使用 result 作为函数中用于返回值的任何变量的名称,因此我将其重命名。

class Service...

  static Offering possiblePreference(Equipment equipment, Region region) {
    Offering result = null;
    var allOfferings = equipment.AllOfferings(region);
    result = allOfferings.FirstOrDefault(o => o.isPreferred);
    if (null != result) return result;
    result = allOfferings.LastOrDefault(o => o.isMatch);
    if (null != result) return result;
    return allOfferings.First();
  }

我现在对 possiblePreference 比较满意,我认为它在领域内以一种有意义的方式清楚地说明了逻辑。我不再需要弄清楚代码的作用才能理解它的意图。

但是,由于我使用的是 C#,我可以使用空合并运算符 (??) 使其更易读。这允许我将多个表达式链接在一起,并返回第一个非空表达式。

class Service...

  static Offering possiblePreference(Equipment equipment, Region region) {
    var allOfferings = equipment.AllOfferings(region);
    return allOfferings.FirstOrDefault(o => o.isPreferred)
      ?? allOfferings.LastOrDefault(o => o.isMatch)
      ?? allOfferings.First();
  }

在将空值视为假值的类型较弱的语言中,可以使用“或”运算符执行相同操作。另一种选择是组合一等函数(但这属于另一个主题)。

现在我回到外层循环,它目前看起来像这样。

class Service...

  var loopStart = equipment.AllOfferings()
    .Select(o => o.Region)
    .Distinct();
  foreach (Region r in loopStart) {
    var possPref = possiblePreference(equipment, r);
    possPref.isPreferred = true;
  }

我可以在管道中使用我的 possiblePreference

class Service...

  var loopStart = equipment.AllOfferings()
    .Select(o => o.Region)
    .Distinct()
    .Select(r => possiblePreference(equipment, r))
    ;
  foreach (Offering o in loopStart) {
    var possPref = possiblePreference(product, r);
    o.isPreferred = true;
  }

请注意将分号放在单独一行中的样式。我经常在较长的管道中这样做,因为它可以更轻松地操作管道。

通过重命名初始循环变量,结果可以清晰易读。

class Service...

  var preferredOfferings = equipment.AllOfferings()
    .Select(o => o.Region)
    .Distinct()
    .Select(r => possiblePreference(equipment, r))
    ;
  foreach (Offering o in preferredOfferings) {
    o.isPreferred = true;
  }

我很乐意就这样结束,但我也可以将 forEach 行为移入管道,如下所示。

class Service...

  equipment.AllOfferings()
    .Select(o => o.Region)
    .Distinct()
    .Select(r => possiblePreference(equipment, r))
    .ToList()
    .ForEach(o => o.isPreferred = true)
    ;

这是一个更有争议的步骤。许多人不喜欢在管道中使用有副作用的函数。这就是为什么我必须使用中间 ToList,因为 ForEachIEnumerable 上不可用。除了副作用问题之外,使用 ToList 还提醒我们,每当我们使用副作用时,我们也会失去管道评估中的任何惰性(这里不是问题,因为管道的全部目的是选择一些对象进行修改)。

但是,无论哪种方式,我发现这比原始循环清晰得多。早期的循环示例相当容易理解,但这个循环需要一些思考才能弄清楚它在做什么。当然,提取 possiblePreference 是使其更清晰的一个重要因素,你可以这样做并保留循环,尽管我当然希望避免修改逻辑以确保避免重复区域。

对航班记录进行分组

通过这个示例,我将查看一些汇总航班延误信息的代码。代码从准时航班性能记录开始,最初来自美国交通部 交通统计局。经过一些初步的处理,生成的数据看起来像这样

[
  {
    "origin":"BOS","dest":"LAX","date":"2015-01-12",
    "number":"25","carrier":"AA","delay":0.0,"cancelled":false
  },
  {
    "origin":"BOS","dest":"LAX","date":"2015-01-13",
    "number":"25","carrier":"AA","delay":0.0,"cancelled":false
  },
 …

这是处理它的循环

export function airportData() {
  const data = flightData();
  const count = {};
  const cancellations = {};
  const totalDelay = {};
  for (let row of data) {
    const airport = row.dest;
    if (count[airport] === undefined) {
      count[airport] = 0;
      cancellations[airport] = 0;
      totalDelay[airport] = 0;
    }
    count[airport]++;
    if (row.cancelled) {
      cancellations[airport]++ ;
    }
    else {
      totalDelay[airport] += row.delay;
    }
  }

  const result = {};
  for (let i in count) {
    result[i] = {};
    result[i].meanDelay = totalDelay[i] / (count[i] - cancellations[i]);
    result[i].cancellationRate = cancellations[i] / count[i];
  }
  return result;
}

此示例使用 Javascript(节点上的 es6),因为现在所有内容都必须用 Javascript 编写。

该循环按目的地机场 (dest) 汇总航班数据,并计算取消率和平均延误。这里的主要活动是按目的地对航班数据进行分组,这非常适合管道中的 分组运算符。因此,我的第一步是创建一个捕获此分组的变量。

import _ from 'underscore';
export function airportData() {
  const data = flightData();
  const working = _.groupBy(data, r => r.dest);
  const count = {};
  const cancellations = {};
  const totalDelay = {};
  for (let row of data) {
    const airport = row.dest;
    if (count[airport] === undefined) {
      count[airport] = 0;
      cancellations[airport] = 0;
      totalDelay[airport] = 0;
    }
    count[airport]++;
    if (row.cancelled) {
      cancellations[airport]++;
    }
    else {
      totalDelay[airport] += row.delay;
    }
  }

  const result = {};
  for (let i in count) {
    result[i] = {};
    result[i].meanDelay = totalDelay[i] / (count[i] - cancellations[i]);
    result[i].cancellationRate = cancellations[i] / count[i];
  }
  return result;
}

关于此步骤,有两点需要注意。首先,我想不出一个好名字,所以我只是称之为 working。其次,虽然 javascript 在 Array 上有一套很好的集合管道运算符,但它缺少分组运算符。我可以自己编写一个,但我会使用 underscore 库,它长期以来一直是 Javascript 世界中一个有用的工具。

count 变量捕获每个目的地机场有多少条航班记录。我可以用 映射操作 在管道中轻松计算它。

export function airportData() {
  const data = flightData();
  const working = _.chain(data)
      .groupBy(r => r.dest)
      .mapObject((val, key) => {return {count: val.length}})
      .value()
    ;
  const count = {};
  const cancellations = {};
  const totalDelay = {};
  for (let row of data) {
    const airport = row.dest;
    if (count[airport] === undefined) {
      count[airport] = 0;
      cancellations[airport] = 0;
      totalDelay[airport] = 0;
    }
    count[airport]++;
    if (row.cancelled) {
      cancellations[airport]++;
    }
    else {
      totalDelay[airport] += row.delay;
    }
  }

  const result = {};
  for (let i in count) {
    result[i] = {};
    result[i].meanDelay = totalDelay[i] / (working[i].count - cancellations[i]);
    result[i].cancellationRate = cancellations[i] / working[i].count;
  }
  return result;
}

要在 underscore 中执行这样的多步骤管道,我必须使用 chain 函数启动管道。这确保管道中的每个步骤都包装在 underscore 中,因此我可以使用 方法链 来构建管道。缺点是我必须在最后使用 value 来从其中获取底层数组。

映射操作不是标准映射,因为它作用于 Javascript 对象的内容,本质上是一个哈希映射,因此映射函数作用于键值对。在 underscore 中,我使用 mapObject 函数来执行此操作。

通常,当我将行为移入管道时,我喜欢完全删除控制变量,但它也起着跟踪所需键的作用,因此我会保留它一段时间,直到我处理完其他计算。

接下来,我将处理取消变量,这次我可以删除它。

export function airportData() {
  const data = flightData();
  const working = _.chain(data)
      .groupBy(r => r.dest)
      .mapObject((val, key) => {
        return {
          count: val.length,
          cancellations: val.filter(r => r.cancelled).length
        }
      })
      .value()
    ;
  const count = {};
  const cancellations = {};
  const totalDelay = {};
  const cancellations = {};
  for (let row of data) {
    const airport = row.dest;
    if (count[airport] === undefined) {
      count[airport] = 0;
      cancellations[airport] = 0;
      totalDelay[airport] = 0;
    }
    count[airport]++;
    if (row.cancelled) {
      cancellations[airport]++;
    }
    else {
      totalDelay[airport] += row.delay;
    }
  }

  const result = {};
  for (let i in count) {
    result[i] = {};
    result[i].meanDelay = totalDelay[i] / (working[i].count - working[i].cancellations);
    result[i].cancellationRate = working[i].cancellations / working[i].count;
  }
  return result;
}

映射函数现在变得相当长,因此我认为现在是时候对它使用 提取方法 了。

export function airportData() {
 const data = flightData();
  const summarize = function(rows) {
    return {
      count: rows.length,
      cancellations: rows.filter(r => r.cancelled).length
    }
  }
  const working = _.chain(data)
    .groupBy(r => r.dest)
    .mapObject((val, key) => summarize(val))
    .value()
    ;

  const count = {};
  const totalDelay = {}
  for (let row of data) {
    const airport = row.dest;
    if (count[airport] === undefined) {
      count[airport] = 0;
      totalDelay[airport] = 0;
    }
    count[airport]++;
    if (row.cancelled) {
    }
    else {
      totalDelay[airport] += row.delay;
    }
  }

  const result = {};
  for (let i in count) {
    result[i] = {};
    result[i].meanDelay = totalDelay[i] / (working[i].count - working[i].cancellations);
    result[i].cancellationRate = working[i].cancellations / working[i].count;
  }
  return result;
}

在整个函数中将函数分配给变量是 javascript 将函数定义嵌套以将其范围限制在 airportData 函数中的方式。我可以想象这个函数可能更有用,但这将是以后要考虑的重构。

现在处理总延误计算。

export function airportData() {
 const data = flightData();
  const summarize = function(rows) {
    return {
      count: rows.length,
      cancellations: rows.filter(r => r.cancelled).length,
      totalDelay: rows.filter(r => !r.cancelled).reduce((acc,each) => acc + each.delay, 0)
    }
  }
  const working = _.chain(data)
    .groupBy(r => r.dest)
    .mapObject((val, key) => summarize(val))
    .value()
    ;

  const count = {};
  const totalDelay = {}
  for (let row of data) {
    const airport = row.dest;
    if (count[airport] === undefined) {
      count[airport] = 0;
      totalDelay[airport] = 0;
    }
    count[airport]++;
    if (row.cancelled) {
    }
    else {
      totalDelay[airport] += row.delay;
    }
  }

  const result = {};
  for (let i in count) {
    result[i] = {};
    result[i].meanDelay = working[i].totalDelay / (working[i].count - working[i].cancellations);
    result[i].cancellationRate = working[i].cancellations / working[i].count;
  }
  return result;
}

lambda 中用于总延误的表达式反映了原始公式,使用 reduce 操作 来计算总和。我经常发现先使用映射会更容易读。

export function airportData() {
  const data = flightData();
  const summarize = function(rows) {
    return {
      count: rows.length,
      cancellations: rows.filter(r => r.cancelled).length,
      totalDelay: rows.filter(r => !r.cancelled).map(r => r.delay).reduce((a,b) => a + b)
    }
  }
  const working = _.chain(data)
    .groupBy(r => r.dest)
    .mapObject((val, key) => summarize(val))
    .value()
    ;

  const count = {};
  for (let row of data) {
    const airport = row.dest;
    if (count[airport] === undefined) {
      count[airport] = 0;
    }
    count[airport]++;
  }

  const result = {};
  for (let i in count) {
    result[i] = {};
    result[i].meanDelay = working[i].totalDelay / (working[i].count - working[i].cancellations);
    result[i].cancellationRate = working[i].cancellations / working[i].count;
  }
  return result;
}

这种重构不是什么大问题,但我越来越喜欢它。该 lambda 也有点长,但还没有达到我认为需要提取它的程度。

我还借此机会用函数名称替换了调用 summarize 的 lambda。

export function airportData() {
  const data = flightData();
  const summarize = function(rows) {
    return {
      count: rows.length,
      cancellations: rows.filter(r => r.cancelled).length,
      totalDelay: rows.filter(r => !r.cancelled).map(r => r.delay).reduce((a,b) => a + b)
    }
  }
  const working = _.chain(data)
    .groupBy(r => r.dest)
    .mapObject(summarize)
    .value()
    ;

  const count = {};
  for (let row of data) {
    const airport = row.dest;
    if (count[airport] === undefined) {
      count[airport] = 0;
    }
    count[airport]++;
  }

  const result = {};
  for (let i in count) {
    result[i] = {};
    result[i].meanDelay = working[i].totalDelay / (working[i].count - working[i].cancellations);
    result[i].cancellationRate = working[i].cancellations / working[i].count;
  }
  return result;
}

现在所有依赖数据都已删除,我准备删除 count

export function airportData() {
  const data = flightData();
  const summarize = function(rows) {
    return {
      count: rows.length,
      cancellations: rows.filter(r => r.cancelled).length,
      totalDelay: rows.filter(r => !r.cancelled).map(r => r.delay).reduce((a,b) => a + b)
    }
  }
  const working = _.chain(data)
    .groupBy(r => r.dest)
    .mapObject(summarize)
    .value()
    ;

  const count = {};
  for (let row of data) {

    const airport = row.dest;
    if (count[airport] === undefined) {
      count[airport] = 0;
    }
    count[airport]++;
  }

  const result = {};
  for (let i in working) {
    result[i] = {};
    result[i].meanDelay = working[i].totalDelay / (working[i].count - working[i].cancellations);
    result[i].cancellationRate = working[i].cancellations / working[i].count;
  }
  return result;
}

现在我将注意力转向第二个循环,它本质上是在执行 映射 来计算它的两个值。

export function airportData() {
  const data = flightData();
  const summarize = function(rows) {
    return {
      count: rows.length,
      cancellations: rows.filter(r => r.cancelled).length,
      totalDelay: rows.filter(r => !r.cancelled).map(r => r.delay).reduce((a,b) => a + b)
    }
  }
  const formResult = function(row) {
    return {
      meanDelay: row.totalDelay / (row.count - row.cancellations),
      cancellationRate: row.cancellations / row.count
    }
  }
  let working = _.chain(data)
    .groupBy(r => r.dest)
    .mapObject(summarize)
    .mapObject(formResult)
    .value()
    ;

  return working;
  let result = {};
  for (let i in working) {
    result[i] = {};
    result[i].meanDelay = working[i].totalDelay / (working[i].count - working[i].cancellations);
    result[i].cancellationRate = working[i].cancellations / working[i].count;
  }
  return result;
}

完成所有这些后,我可以对 working 使用 内联临时变量,并进行一些重命名和整理。

export function airportData() {
  const data = flightData();
  const summarize = function(flights) {
    return {
      numFlights:       flights.length,
      numCancellations: flights.filter(f => f.cancelled).length,
      totalDelay:       flights.filter(f => !f.cancelled).map(f => f.delay).reduce((a,b) => a + b)
    }
  }
  const formResult = function(airport) {
    return {
      meanDelay:        airport.totalDelay / (airport.numFlights - airport.numCancellations),
      cancellationRate: airport.numCancellations / airport.numFlights
    }
  }
  return _.chain(data)
    .groupBy(r => r.dest)
    .mapObject(summarize)
    .mapObject(formResult)
    .value()
    ;
}

正如经常发生的那样,最终函数的可读性很大程度上来自提取函数。但我发现分组运算符在很大程度上阐明了函数的目的,并有助于设置提取。

如果数据来自关系数据库并且我遇到性能问题,那么这种重构还有另一个潜在的好处。通过从循环重构到集合管道,我以更类似于 SQL 的形式表示转换。在这种情况下,我可能会从数据库中提取大量数据,但重构后的代码使考虑将分组和第一级汇总逻辑移入 SQL 变得更容易,这将减少我需要通过网络传输的数据量。通常,我更喜欢将逻辑保留在应用程序代码中而不是 SQL 中,因此我会将此类操作视为性能优化,并且只有在我能够衡量到显著的性能改进时才会这样做。但这再次强调了 在拥有清晰的代码时更容易进行优化,这就是为什么我认识的所有性能专家都强调以清晰度为基础来构建高性能代码的重要性。

标识符

在下一个示例中,我将查看一些检查一个人是否具有所有必需标识符的代码。系统通常依赖于通过某种希望是唯一的 ID(例如客户 ID)来识别人员。在许多领域,你必须处理许多不同的识别方案,并且一个人应该具有多个方案的标识符。因此,镇政府可能会期望一个人拥有镇 ID、州 ID 和国家 ID。

图 2:一个人拥有不同方案的标识符的数据模型

这种情况下的数据结构非常简单。Person 类有一个标识符对象的集合。标识符有一个用于方案的字段和一些值。但通常还有其他约束,这些约束不能仅通过数据模型来强制执行,这些约束由验证函数(如以下函数)检查。

class Person...

  def check_valid_ids required_schemes, note: nil
    note ||= Notification.new
    note.add_error "has no ids"  if @ids.size < 1

    used = []
    found_required = []
    dups = []

    for id in @ids
      next if id.void?
      if used.include?(id.scheme)
        dups << id.scheme
      else
        for req in required_schemes
          if id.scheme == req
            found_required << req
            required_schemes.delete req
            next
          end
        end
      end
      used << id.scheme
    end
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    if required_schemes.size > 0
      missing_names = ""
      for req in required_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

此示例使用 Ruby,因为我喜欢用 Ruby 编程

还有几个其他对象支持此循环。Identifier 类知道它的方案、值以及它是否为空 - 意味着它在逻辑上已删除(但仍保留在数据库中)。还有一个 通知 用于跟踪任何错误。

class Identifier
  attr_reader :scheme, :value
  def void?
    …

class Notification
  def add_error e
    …

我对这个例程最大的反感是循环同时执行两件事。它既查找重复的标识符(收集在 dups 中),也查找缺少的必需方案(收集在 required_schemes 中)。程序员在面对对同一对象集合执行两件事时,决定在同一个循环中执行这两件事的情况很常见。原因之一是设置循环所需的代码,写两次似乎很可惜。现代循环结构和管道消除了这种负担。更糟糕的原因是性能问题。当然,许多性能热点都涉及循环,并且在某些情况下,可以融合循环以改进性能。但这些仅仅是我们编写的循环中的一小部分,因此我们应该遵循编程的通常原则。除非你遇到可衡量的重大性能问题,否则应将清晰度放在性能之上。如果你遇到这样的问题,那么修复它优先于清晰度,但这种情况很少见。

除非你遇到可衡量的重大性能问题,否则应将清晰度放在性能之上。

因此,面对一个执行两件事的循环,我毫不犹豫地复制循环以提高清晰度。性能分析极少会导致我撤销这种重构。

因此,我的第一步是使用一种我称之为“拆分循环”的重构。当我这样做时,我首先获取循环和将它连接到一个连贯的代码块中的代码,并对其应用 提取方法

class Person...

  def check_valid_ids required_schemes, note: nil
    note ||= Notification.new
    note.add_error "has no ids"  if @ids.size < 1
    return inner_check_valid_ids required_schemes, note: note
  end
  def inner_check_valid_ids required_schemes, note: nil
    used = []
    found_required = []
    dups = []

    for id in @ids
      next if id.void?
      if used.include?(id.scheme)
        dups << id.scheme
      else
        for req in required_schemes
          if id.scheme == req
            found_required << req
            required_schemes.delete req
            next
          end
        end
      end
      used << id.scheme
    end
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    if required_schemes.size > 0
      missing_names = ""
      for req in required_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

此提取的方法执行两件事,因此现在我想复制它以形成两个独立方法的脚手架,每个方法只执行一件事。如果我复制它并分别调用它们,那么我的累积通知将收到两倍的错误,我可以通过从每个副本中删除无关的更新来避免这种情况。

class Person...

  def check_valid_ids required_schemes, note: nil
    note ||= Notification.new
    note.add_error "has no ids"  if @ids.size < 1
    check_no_duplicate_ids required_schemes, note: note
    check_all_required_schemes required_schemes, note: note
  end
  def check_no_duplicate_ids required_schemes, note: nil
    used = []
    found_required = []
    dups = []

    for id in @ids
      next if id.void?
      if used.include?(id.scheme)
        dups << id.scheme
      else
        for req in required_schemes
          if id.scheme == req
            found_required << req
            required_schemes.delete req
            next
          end
        end
      end
      used << id.scheme
    end
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    if required_schemes.size > 0
      missing_names = ""
      for req in required_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

  def check_all_required_schemes required_schemes, note: nil
    used = []
    found_required = []
    dups = []

    for id in @ids
      next if id.void?
      if used.include?(id.scheme)
        dups << id.scheme
      else
        for req in required_schemes
          if id.scheme == req
            found_required << req
            required_schemes.delete req
            next
          end
        end
      end
      used << id.scheme
    end
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    if required_schemes.size > 0
      missing_names = ""
      for req in required_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

删除双重更新很重要,这样我的所有测试在重构时都能继续通过。

结果相当难看,但现在我可以独立地处理每个方法,删除任何与每个方法的目的无关的内容。

重构无重复项检查

我将从无重复的情况开始,我可以分几步剪切掉几块代码,在每一步之后进行测试以确保我没有犯错误。我首先删除使用 required_schemes 的代码

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    used = []
    found_required = []
    dups = []

    for id in @ids
      next if id.void?
      if used.include?(id.scheme)
        dups << id.scheme
      else
        for req in required_schemes
          if id.scheme == req
            found_required << req
            required_schemes.delete req
            next
          end
        end
      end
      used << id.scheme
    end
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    if required_schemes.size > 0
      missing_names = ""
      for req in required_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
    end

    return note
  end

然后我删除了不必要的条件分支

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    used = []
    found_required = []
    dups = []

    for id in @ids
      next if id.void?
      if used.include?(id.scheme)
        dups << id.scheme
      else
        for req in required_schemes
          if id.scheme == req
            found_required << req
            required_schemes.delete req
            next
          end
        end
      end
      used << id.scheme
    end
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    return note
  end

此时,我可以(也许应该)删除现在不再需要的 required_schemes 参数。我没有这样做,你将在最后看到它被整理出来。

我执行我通常的 提取变量

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    used = []
    dups = []

    input = @ids
    for id in input
      next if id.void?
      if used.include?(id.scheme)
        dups << id.scheme
      end
      used << id.scheme
    end
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    return note
  end

然后,我可以向输入变量添加一个 过滤器,并删除跳过空标识符的行。

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    used = []
    dups = []

    input = @ids.reject{|id| id.void?}
    for id in input
      next if id.void?
      if used.include?(id.scheme)
        dups << id.scheme
      end
      used << id.scheme
    end
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    return note
  end

进一步查看循环,我发现它使用的是方案而不是 ID,因此我可以添加管道步骤来将 ID 映射 到方案。

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    used = []
    dups = []

    input = @ids
      .reject{|id| id.void?}
      .map {|id| id.scheme}
    for scheme in input
      if used.include?(scheme)
        dups << scheme
      end
      used << scheme
    end
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    return note
  end

此时,我已经将循环主体简化为简单的删除重复行为。有一种管道方法可以查找重复项,即按自身对方案进行 分组,并 过滤 那些出现不止一次的方案。

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    used = []
    dups = []

    input = @ids
      .reject{|id| id.void?}
      .map {|id| id.scheme}
      .group_by {|s| s}
      .select {|k,v| v.size > 1}
      .keys
    for scheme in input
      if used.include?(scheme)
        dups << scheme
      end
      used << scheme
    end
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    return note
  end

现在管道的输出是重复项,因此我可以删除输入变量并将管道分配给变量(并删除现在不再需要的 used 变量)。

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    used = []
    dups = []

    dups = @ids
      .reject{|id| id.void?}
      .map {|id| id.scheme}
      .group_by {|s| s}
      .select {|k,v| v.size > 1}
      .keys
    for scheme in input
      dups << scheme
      used << scheme
    end
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    return note
  end

这给了我们一个不错的管道,但它有一个令人不安的因素。管道中的最后三个步骤是为了删除重复项,但这种知识在我的脑海中,而不是在代码中。我需要通过使用 提取方法 将其移入代码。

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    dups = @ids
      .reject{|id| id.void?}
      .map {|id| id.scheme}
      .duplicates
    
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end

    return note
  end

class Array…

  def duplicates
    self
      .group_by {|s| s}
      .select {|k,v| v.size > 1}
      .keys
  end

在这里,我使用了 Ruby 的能力,可以向现有类添加方法(称为猴子补丁)。我也可以在最新的 Ruby 版本中使用 Ruby 的 refinement 特性。但是,许多面向对象语言不支持猴子补丁,在这种情况下,我必须使用本地定义的函数,类似于以下函数

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    schemes = @ids
      .reject{|id| id.void?}
      .map {|id| id.scheme}
    
    if duplicates(schemes).size > 0
      note.add_error "duplicate schemes: " + duplicates(schemes).join(", ")
    end

    return note
  end
  def duplicates anArray
    anArray
      .group_by {|s| s}
      .select {|k,v| v.size > 1}
      .keys
  end    

在管道中,将方法定义在数组上比定义在 person 对象上效果更好。但通常我们无法将方法定义在数组上,因为我们的语言不支持猴子补丁,或者项目标准不允许这样做,或者该方法不够通用,无法放在通用的列表类上。在这种情况下,面向对象的方案会阻碍我们,而函数式方案(不将方法绑定到对象)则更有效。

每当我有这样的局部变量时,我都会考虑使用 将临时变量替换为查询 将变量转换为方法,最终得到类似这样的结果。

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    if duplicate_identity_schemes.size > 0
      note.add_error "duplicate schemes: " + duplicate_identity_schemes.join(", ")
    end

    return note
  end
  def duplicate_identity_schemes
     @ids
      .reject{|id| id.void?}
      .map {|id| id.scheme}
      .duplicates
  end

我根据 duplicate_identity_schemes 行为是否可能对 person 类中的其他方法有用来做出决定。但尽管我更倾向于创建查询方法,但在这种情况下,我更倾向于将其保留为局部变量。

重构所有必需方案的检查

现在我已经清理了检查是否存在重复项的代码,可以开始处理检查是否包含所有必需方案的代码。这是当前的方法。

class Person...

  def check_all_required_schemes required_schemes, note: nil
    used = []
    found_required = []
    dups = []

    for id in @ids
      next if id.void?
      if used.include?(id.scheme)
        dups << id.scheme
      else
        for req in required_schemes
          if id.scheme == req
            found_required << req
            required_schemes.delete req
            next
          end
        end
      end
      used << id.scheme
    end
    
    if dups.size > 0
    end

    if required_schemes.size > 0
      missing_names = ""
      for req in required_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

与之前的方法一样,我的第一步是删除所有与检查重复项相关的代码。

class Person...

  def check_all_required_schemes required_schemes, note: nil
    used = []
    found_required = []
    dups = []

    for id in @ids
      next if id.void?
      if used.include?(id.scheme)
        dups << id.scheme
      else
        for req in required_schemes
          if id.scheme == req
            found_required << req
            required_schemes.delete req
            next
          end
        end
      end
      used << id.scheme
    end
    
    if dups.size > 0
    end

    if required_schemes.size > 0
      missing_names = ""
      for req in required_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

为了深入了解这个函数,我首先查看 found_required 变量。与检查重复项的情况一样,它主要关注我们拥有非空标识符的方案,因此我倾向于首先将方案捕获到一个变量中,并使用这些方案而不是 id。

class Person...

  def check_all_required_schemes required_schemes, note: nil
    found_required = []
    schemes = @ids
      .reject{|i| i.void?}
      .map {|i| i.scheme}

    for s in schemes
      next if id.void?
      for req in required_schemes
        if s == req
          found_required << req
          required_schemes.delete req
          next
        end
      end
    end


    if required_schemes.size > 0
      missing_names = ""
      for req in required_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

found_required 的目的是捕获同时存在于 required_schemes 列表中和我们的 id 中的方案。在我看来,这听起来像是集合的交集,而这应该是在任何自尊的集合上都能找到的函数。因此,我应该能够使用该函数来确定 found_required

class Person...

  def check_all_required_schemes required_schemes, note: nil
    found_required = []
    schemes = @ids
      .reject{|i| i.void?}
      .map {|i| i.scheme}

    for s in schemes
      for req in required_schemes
        if s == req
          found_required << req
          required_schemes.delete req
          next
        end
      end
    end
    found_required = schemes & required_schemes


    if required_schemes.size > 0
      missing_names = ""
      for req in required_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

遗憾的是,这个更改导致测试失败。现在我仔细查看代码,发现 found_required 在后面的代码中根本没有使用,它是一个僵尸变量,可能曾经被用来做某事,但后来被放弃了,而变量却没有从代码中删除。因此,我撤回了刚刚做出的更改并将其删除。

class Person...

  def check_all_required_schemes required_schemes, note: nil
    found_required = []
    schemes = @ids
      .reject{|i| i.void?}
      .map {|i| i.scheme}

    for s in schemes
      for req in required_schemes
        if s == req
          found_required << req
          required_schemes.delete req
          next
        end
      end
    end


    if required_schemes.size > 0
      missing_names = ""
      for req in required_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

现在我看到循环正在从参数 required_schemes 中删除元素。修改参数对我来说是一个严格的禁忌,除非它是一个收集参数(例如注释)。因此,我立即应用 删除对参数的赋值

class Person...

  def check_all_required_schemes required_schemes, note: nil
    missing_schemes = required_schemes.dup
    schemes = @ids
      .reject{|i| i.void?}
      .map {|i| i.scheme}

    for s in schemes
      for req in required_schemes
        if s == req
          missing_schemes.delete req
          next
        end
      end
    end


    if missing_schemes.size > 0
      missing_names = ""
      for req in missing_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

这样做也揭示了循环正在从它正在枚举的列表中删除项,这比修改参数更糟糕。

现在已经澄清了这一点,我可以看到集合操作是合适的,但我需要做的是从必需列表中删除我们拥有的方案,使用集合差操作。

class Person...

  def check_all_required_schemes required_schemes, note: nil
    missing_schemes = required_schemes.dup
    schemes = @ids
      .reject{|i| i.void?}
      .map {|i| i.scheme}

    missing_schemes = required_schemes - schemes

    for s in schemes
      for req in required_schemes
        if s == req
          missing_schemes.delete req
          next
        end
      end
    end



    if missing_schemes.size > 0
      missing_names = ""
      for req in missing_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

现在我查看第二个循环,它用于形成错误消息。它只是将方案转换为字符串,并用逗号连接它们,这是字符串连接操作的工作。

class Person...

  def check_all_required_schemes required_schemes, note: nil
    schemes = @ids
      .reject{|i| i.void?}
      .map {|i| i.scheme}
    missing_schemes = required_schemes - schemes

    if missing_schemes.size > 0
      missing_names = missing_schemes.join(", ")
      for req in missing_schemes
        missing_names += (missing_names.size > 0) ? ", " + req.to_s : req.to_s
      end
      note.add_error "missing schemes: " + missing_names
    end

    return note
  end

合并两个方法

现在两种方法都已清理,它们只做一件事,并且清楚地表明了它们在做什么。它们都需要非空标识符的方案列表,因此我倾向于使用 提取方法

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    dups = @ids
      .reject{|id| id.void?}
      .map {|id| id.scheme}
      .duplicates
    dups = identity_schemes.duplicates
    if dups.size > 0
      note.add_error "duplicate schemes: " + dups.join(", ")
    end
    return note
  end

  def check_all_required_schemes required_schemes, note: nil
    schemes = @ids
      .reject{|i| i.void?}
      .map {|i| i.scheme}
    missing_schemes = required_schemes - identity_schemes
    if missing_schemes.size > 0
      missing_names = missing_schemes.join(", ")
      note.add_error "missing schemes: " + missing_names
    end
    return note
  end
  
  def identity_schemes
    @ids
      .reject{|i| i.void?}
      .map {|i| i.scheme}
  end

然后我想进行一些小的清理。首先,我通过检查集合的大小来测试它是否为空。我总是更喜欢更能揭示意图的空方法。

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    dups = identity_schemes.duplicates
    unless dups.empty?
      note.add_error "duplicate schemes: " + dups.join(", ")
    end
    return note
  end
  
  def check_all_required_schemes required_schemes, note: nil
    missing_schemes = required_schemes - identity_schemes
    unless missing_schemes.empty?
      missing_names = missing_schemes.join(", ")
      note.add_error "missing schemes: " + missing_names
    end
    return note
  end

我还没有给这个重构命名,它应该类似于“用揭示意图的方法替换揭示实现的方法”。

missing_names 变量没有发挥作用,因此我将使用 内联临时变量 对其进行处理。

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    dups = identity_schemes.duplicates
    unless dups.empty?
      note.add_error "duplicate schemes: " + dups.join(", ")
    end
    return note
  end
  
  def check_all_required_schemes required_schemes, note: nil
    missing_schemes = required_schemes - identity_schemes
    unless missing_schemes.empty?
      missing_names = missing_schemes.join(", ")
      note.add_error "missing schemes: " + missing_schemes.join(", ")
    end
    return note
  end

我还想将这两个方法都转换为使用单行条件语法。

class Person...

  def check_no_duplicate_ids required_schemes, note: nil
    dups = identity_schemes.duplicates
    unless dups.empty?
    note.add_error "duplicate schemes: " + dups.join(", ") unless dups.empty?
    end
    return note
  end
  
  def check_all_required_schemes required_schemes, note: nil
    missing_schemes = required_schemes - identity_schemes
    unless missing_schemes.empty?
    note.add_error "missing schemes: " + missing_schemes.join(", ")  unless missing_schemes.empty?
    end
    return note
  end

同样,还没有为它定义重构,它将是一个非常特定于 Ruby 的重构。

有了这些,我认为这些方法不再有价值,因此我将它们内联,并将提取的 identity_schemes 方法内联回调用者。

class Person...

  def check_valid_ids required_schemes, note: nil
    note ||= Notification.new
    note.add_error "has no ids"  if @ids.size < 1
    identity_schemes = @ids.reject{|i| i.void?}.map {|i| i.scheme}
    dups = identity_schemes.duplicates
    note.add_error("duplicate schemes: " + dups.join(", ")) unless dups.empty?
    missing_schemes = required_schemes - identity_schemes
    note.add_error "missing schemes: " + missing_schemes.join(", ")  unless missing_schemes.empty?
    return note
  end

最终方法比我通常使用的要长一些,但我喜欢它的凝聚力。如果它变得更大,我想将其拆分,也许可以使用 用方法对象替换方法 即使它很长,我也发现它在传达验证正在检查的错误方面更加清晰。

最后的想法

这套重构示例到此结束。我希望它能让你对集合管道如何能够阐明操作集合的代码逻辑有一个很好的了解,以及如何将循环重构为集合管道通常非常简单。

与任何重构一样,也有类似的逆向重构,可以将集合管道转换为循环,但我很少这样做。

如今,大多数现代语言都提供了一流的函数和包含集合管道所需操作的集合库。如果你不习惯使用集合管道,将遇到的循环重构为这种形式是一个很好的练习。如果你发现最终的管道不如原始循环清晰,你可以在完成后随时撤销重构。即使你撤销了重构,这个练习也能让你学到很多关于这种技术的知识。我使用这种编程模式已经很长时间了,发现它是一种帮助我阅读自己代码的宝贵方法。因此,我认为探索它是值得的,看看你的团队是否会得出类似的结论。


脚注

1: 实际上,我的第一步是考虑对循环应用 提取方法,因为将循环隔离到它自己的函数中通常更容易操作。

2: 对我来说,看到 map 运算符 被称为“Select”很奇怪。原因是 C# 中的管道方法来自 Linq,Linq 的主要目标是抽象数据库访问,因此方法名称的选择与 SQL 类似。“Select”是 SQL 中的投影运算符,当你将其视为选择列时,这很有意义,但如果你将其视为使用函数映射,则是一个奇怪的名称。

3: 当然,这不是所有可以进行集合管道的语言的完整列表,因此我预计会收到通常的抱怨,说我没有展示 Javascript、Scala 或 Whatever++。我不希望这里有一个庞大的语言列表,只需要一个足够多样的小集合,以传达集合管道在不熟悉的语言中易于理解的概念。

4: 在某些情况下,它需要是一个短路运算符,尽管这里并非如此。

5: 我经常在负布尔值中发现这种情况。这是因为否定 (!) 位于表达式的开头,而谓词 (isEmpty) 位于表达式末尾。如果两个之间有任何实质性的表达式,结果就很难解析。(至少对我来说是这样。)

6: 我还没有将它放入运算符目录中。

7: 如果我使用的是没有检测通过谓词的最后一个项目的运算符的语言,我可以先将其反转,然后检测第一个项目。

致谢

Kit Eason 帮助使 F# 示例更具惯用性。Les Ramer 帮助我改进了我的 C#。Richard Warburton 修正了我 Java 中的一些措辞错误。Daniel Sandbecker 发现了 ruby 示例中的错误。Andrew Kiellor、Bruno Trecenti、David Johnston、Duncan Cragg、Karel Alfonso、Korny Sietsma、Matteo Vaccari、Pete Hodgson、Piyush Srivastava、Scott Robinson、Steven Lowe 和 Vijay Aravamudhan 在 Thoughtworks 邮件列表中讨论了本文的草稿。

重大修订

2015 年 7 月 14 日: 添加了标识符示例和最终想法

2015 年 7 月 7 日: 添加了分组航班数据示例

2015 年 6 月 30 日: 添加了设备提供示例。

2015 年 6 月 25 日: 添加了嵌套循环部分

2015 年 6 月 23 日: 发布了第一部分