如何消除for循环中的重复代码？

Question

我在同一个里实现了两个成员函数class:

  private static void getRequiredTag(Context context) throws IOException
  {
    //repeated begin
    for (Record record : context.getContext().readCacheTable("subscribe")) {
      String traceId = record.get("trace_id").toString();
      if (traceSet.contains(traceId) == false)
        continue;
      String tagId = record.get("tag_id").toString();
      try {
        Integer.parseInt(tagId);
      } catch (NumberFormatException e) {
        context.getCounter("Error", "tag_id not a number").increment(1);
        continue;
      }
      //repeated end
      tagSet.add(tagId);
    }
  }

  private static void addTagToTraceId(Context context) throws IOException
  {
    //repeated begin
    for (Record record : context.getContext().readCacheTable("subscribe")) {
      String traceId = record.get("trace_id").toString();
      if (traceSet.contains(traceId) == false)
        continue;
      String tagId = record.get("tag_id").toString();
      try {
        Integer.parseInt(tagId);
      } catch (NumberFormatException e) {
        context.getCounter("Error", "tag_id not a number").increment(1);
        continue;
      }
      //repeated end
      Vector<String> ret = traceListMap.get(tagId);
      if (ret == null) {
        ret = new Vector<String>();
      }
      ret.add(traceId);
      traceListMap.put(tagId, ret);
    }    
  }

我将在另外两个成员函数中调用这两个成员函数（所以我不能将它们合并为一个函数）：

private static void A()
{
  getRequiredTag()
}
private static void B()
{
  getRequiredTag()
  addTagToTraceId()
}

tagSet 是 java.util.Set，traceListMap 是 java.util.Map。

我知道DRY principle而且我很想消除重复代码，所以我来了这个代码：

  private static void getTraceIdAndTagIdFromRecord(Record record, String traceId, String tagId) throws IOException
  {
    traceId = record.get("trace_id").toString();
    tagId = record.get("tag_id").toString();
  }

  private static boolean checkTagIdIsNumber(String tagId)
  {
    try {
      Integer.parseInt(tagId);
    } catch (NumberFormatException e) {
      return false;
    }
    return true;
  }

  private static void getRequiredTag(Context context) throws IOException
  {
    String traceId = null, tagId = null;
    for (Record record : context.getContext().readCacheTable("subscribe")) {
      getTraceIdAndTagIdFromRecord(record, traceId, tagId);
      if (traceSet.contains(traceId) == false)
        continue;
      if (!checkTagIdIsNumber(tagId))
      {
        context.getCounter("Error", "tag_id not a number").increment(1);
        continue;
      }
      tagSet.add(tagId);
    }
  }

  private static void addTagToTraceId(Context context) throws IOException
  {
    String traceId = null, tagId = null;
    for (Record record : context.getContext().readCacheTable("subscribe")) {
      getTraceIdAndTagIdFromRecord(record, traceId, tagId);
      if (traceSet.contains(traceId) == false)
        continue;
      if (!checkTagIdIsNumber(tagId))
      {
        context.getCounter("Error", "tag_id not a number").increment(1);
        continue;
      }
      Vector<String> ret = traceListMap.get(tagId);
      if (ret == null) {
        ret = new Vector<String>();
      }
      ret.add(traceId);
      traceListMap.put(tagId, ret);
    }    
  }

看来我得到了一个新的重复...在那种情况下我不知道如何消除重复，有人可以给我一些建议吗？

更新 2015-5-13 21:15:12:

有些人给出了一个布尔参数来消除重复，但我知道 Robert C. Martin's Clean Code Tip #12: Eliminate Boolean Arguments.（您可以 google 获取更多详细信息）。

你能对此发表一些评论吗？

Answer 1

试试这个方法

private static void imYourNewMethod(Context context,Boolean isAddTag){
        String traceId = null, tagId = null;
        for (Record record : context.getContext().readCacheTable("subscribe")) {
          getTraceIdAndTagIdFromRecord(record, traceId, tagId);
          if (traceSet.contains(traceId) == false)
            continue;
          if (!checkTagIdIsNumber(tagId))
          {
            context.getCounter("Error", "tag_id not a number").increment(1);
            continue;
          }
        if(isAddTag){
            Vector<String> ret = traceListMap.get(tagId);
              if (ret == null) {
                ret = new Vector<String>();
              }
          ret.add(traceId);
          traceListMap.put(tagId, ret);
        }else{
            tagSet.add(tagId);
        }
    }

调用此方法，再传一个参数boolean true，否则false即可得到。

Answer 2

在您第二次尝试时，tagId 似乎始终为空。

尽管如此，一种方法是将收集 tagId 的代码（这在两种方法中似乎相同）提取到它自己的方法中。然后，在这两种方法中的每一种中，只需遍历返回的 tagId 的集合并对它们执行不同的操作。

for (String tagId : getTagIds(context)) {
// do method specific logic
}

编辑

现在我注意到你在第二种方法中也使用了traceId。原理还是一样的，只是在一个单独的方法中收集记录，然后在两个方法中迭代它们（通过从记录中获取 tagId 和 traceId）。

使用 lambda 的解决方案是最优雅的解决方案，但如果没有它们，它涉及创建单独的接口和两个匿名类，这对于这个用例来说太冗长了（老实说，在这里我宁愿使用布尔值参数比没有 lambda 的策略）。

Answer 3

更改的部分需要 String tagId 和 String traceId 的值，因此我们将从 提取采用这些参数的接口 开始：

public static class PerformingInterface {
     void accept(String tagId, String traceId);
}

然后将公共部分提取到这个方法中：

  private static void doSomething(Context context, PerformingInterface perform) throws IOException
  {
    String traceId = null, tagId = null;
    for (Record record : context.getContext().readCacheTable("subscribe")) {
      getTraceIdAndTagIdFromRecord(record, traceId, tagId);
      if (traceSet.contains(traceId) == false)
        continue;
      if (!checkTagIdIsNumber(tagId))
      {
        context.getCounter("Error", "tag_id not a number").increment(1);
        continue;
      }
      perform.accept(tagId, traceId);
    }    
  }

然后以两种不同的方式调用此方法：

private static void getRequiredTag(Context context) throws IOException {
    doSomething(context, new PerformingInterface() {
         @Override public void accept(String tagId, String traceId) {
              tagSet.add(tagId);
         }
    });
}

private static void addTagToTraceId(Context context) throws IOException {
    doSomething(context, new PerformingInterface() {
         @Override public void accept(String tagId, String traceId) {
             Vector<String> ret = traceListMap.get(tagId);
             if (ret == null) {
                 ret = new Vector<String>();
             }
             ret.add(traceId);
             traceListMap.put(tagId, ret);
         }
    });
}

注意我这里使用的是lambdas，这是Java8的一个特性（BiConsumer也是Java8中定义的函数式接口），但是完全可以在 Java 7 及以下完成同样的事情，它只需要一些更冗长的代码。

您的代码存在一些其他问题：

事情太多了 static
Vectorclass比较老，更推荐使用ArrayList（如果需要同步，包在Collections.synchronizedList里）
始终使用牙套，即使是单线牙套

Answer 4

您可以使用流（尚未测试）：

private static Stream<Record> validRecords(Context context) throws IOException {
    return context.getContext().readCacheTable("subscribe").stream()
        .filter(r -> {
            if (!traceSet.contains(traceId(r))) {
                return false;
            }
            try {
                Integer.parseInt(tagId(r));
                return true;
            } catch (NumberFormatException e) {
                context.getCounter("Error", "tag_id not a number").increment(1);
                return false;
            }
        });
}

private static String traceId(Record record) {
    return record.get("trace_id").toString();
}

private static String tagId(Record record) {
    return record.get("tag_id").toString();
}

然后可以这样做：

private static void getRequiredTag(Context context) throws IOException {
    validRecords(context).map(r -> tagId(r)).forEach(tagSet::add);
}

private static void addTagToTraceId(Context context) throws IOException {
    validRecords(context).forEach(r -> {
        String tagId = tagId(r);
        Vector<String> ret = traceListMap.get(tagId);
        if (ret == null) {
            ret = new Vector<String>();
        }
        ret.add(traceId(r));
        traceListMap.put(tagId, ret);
    });
}

如何消除for循环中的重复代码？

How to eliminate repeat code in a for-loop?

java

dry