我是高级前端开发人员，使用Babel ES6进行编码。我们的应用程序的一部分进行了API调用，根据我们从API调用获得的数据模型，需要填写某些表格。

那些表单存储在一个双向链接的列表中（如果后端说一些数据无效，我们可以迅速将用户带回到他们弄乱的一页，然后将其弄乱只需通过修改列表就可以重新回到目标。）

无论如何，有很多用于添加页面的函数，我想知道自己是否太聪明了。这只是一个基本的概述-实际的算法要复杂得多，有大量不同的页面和页面类型，但这将为您提供示例。

我认为这是新手程序员将如何处理的方法。 



 export const addPages = (apiData) => {
   let pagesList = new PagesList(); 

   if(apiData.pages.foo){
     pagesList.add('foo', apiData.pages.foo){
   }

   if (apiData.pages.arrayOfBars){
      let bars = apiData.pages.arrayOfBars;
      bars.forEach((bar) => {
         pagesList.add(bar.name, bar.data);
      })
   }

   if (apiData.pages.customBazes) {
      let bazes = apiData.pages.customBazes;
      bazes.forEach((baz) => {
         pagesList.add(customBazParser(baz)); 
      })
   } 

   return pagesList;
}


现在，为了更易于测试，我已经采用了所有这些if语句，并将它们分开，成为独立的函数，然后将它们映射。

现在，可测试是一回事，但可读性也是一回事，我想知道我是否在这里使事情变得不那么可读。 

 // file: '../util/functor.js'

export const Identity = (x) => ({
  value: x,
  map: (f) => Identity(f(x)),
})

// file 'addPages.js' 

import { Identity } from '../util/functor'; 

export const parseFoo = (data) => (list) => {
   list.add('foo', data); 
}

export const parseBar = (data) => (list) => {
   data.forEach((bar) => {
     list.add(bar.name, bar.data)
   }); 
   return list; 
} 

export const parseBaz = (data) => (list) => {
   data.forEach((baz) => {
      list.add(customBazParser(baz)); 
   })
   return list;
}


export const addPages = (apiData) => {
   let pagesList = new PagesList(); 
   let { foo, arrayOfBars: bars, customBazes: bazes } = apiData.pages; 

   let pages = Identity(pagesList); 

   return pages.map(foo ? parseFoo(foo) : x => x)
               .map(bars ? parseBar(bars) : x => x)
               .map(bazes ? parseBaz(bazes) : x => x)
               .value

}


这是我的关注点。对我来说，底层更有条理。代码本身分为较小的块，可以单独进行测试。但是我在想：如果我必须作为一个初级开发人员来阅读它，而不使用诸如使用Identity仿函数，currying或三元语句之类的概念，那么我是否能够理解后者的解决方案在做什么？有时候以“错误，容易”的方式做事更好吗？

评论

作为一个只在JS方面有10年自我教学经验的人，我会以为自己是Jr.，而您在Babel ES6中迷失了我。
OMG-自1999年以来一直活跃于行业，自1983年以来一直从事编码，您是目前最有害的开发人员。您认为“聪明”的人实际上被称为“昂贵的”，“难以维护的”和“错误的来源”，并且在商业环境中没有地位。第一个例子很简单，易于理解，并且可以正常工作；而第二个例子很复杂，很难理解，而且无法证明正确。请停止做这种事情。它不是更好，除非在某些学术意义上可能不适用于现实世界。
我只想在这里引用Brian Kerninghan的话：“每个人都知道调试首先要比编写程序难一倍。因此，如果您在编写程序时表现得尽可能聪明，那么如何调试它？ ” -en.wikiquote.org/wiki/Brian_Kernighan /“编程风格的元素”，第二版，第2章。
@Logister Coolness不再是简单性的主要目标。此处的反对意见是无谓的复杂性，这是正确性的敌人（肯定是首要问题），因为它使代码更难以推理，并且更有可能包含意外的极端情况。考虑到我先前所说的对实际上更容易测试的说法的怀疑，因此我没有看到任何令人信服的论点。类似于安全性最低特权规则，也许会有一条经验法则说，应该警惕使用强大的语言功能来完成简单的事情。
您的代码看起来像初级代码。我希望大四生写第一个例子。

#1 楼

在您的代码中，您进行了多项更改：


对pages中的访问字段进行结构化分配是一个不错的更改。
提取parseFoo()函数等可能是一个不错的更改。
引入函子非常……令人困惑。

这里最令人困惑的部分是如何混合使用函数式和命令式编程。有了functor，您并没有真正转换数据，而是在使用它通过各种功能传递可变列表。看起来这不是一个非常有用的抽象，我们已经有了一些变量。应该应该抽象的东西（如果存在则仅对其进行分析）仍然明确地存在于您的代码中，但是现在我们必须考虑一下。例如，parseFoo(foo)将返回一个函数在某种程度上并不明显。 JavaScript没有静态类型系统来通知您这是否合法，因此，如果没有更好的名称，此类代码实际上很容易出错（makeFooParser(foo)？）。在这种混淆中，我看不到任何好处。

相反，我希望看到的是：



 if (foo) parseFoo(pages, foo);
if (bars) parseBar(pages, bars);
if (bazes) parseBaz(pages, bazes);
return pages;


但这也不理想，因为从呼叫站点尚不清楚这些项目将被添加到页面列表中。相反，如果解析函数是纯函数并返回一个我们可以显式添加到页面的（可能为空）列表，那可能会更好：

 pages.addAll(parseFoo(foo));
pages.addAll(parseBar(bars));
pages.addAll(parseBaz(bazes));
return pages;


附加的好处：现在，将有关将项目为空时的操作的逻辑移到各个解析函数中。如果不合适，您仍然可以引入条件句。现在，pages列表的可变性被整合到一个函数中，而不是将其分散到多个调用中。与具有诸如Monad之类的有趣名称的抽象相比，避免非局部突变是函数编程的重要组成部分。

所以是的，您的代码太聪明了。请运用您的聪明才智，而不是编写聪明的代码，而是寻找聪明的方法来避免需要公然的聪明才智。最好的设计看起来并不花哨，但是对于任何看到它们的人来说都是显而易见的。良好的抽象可以简化编程，而不必添加我必须首先在脑海中解开的多余层（在这里，要确定函子等效于变量，并且可以有效地消除它）。

请：如果有疑问，请保持您的代码简单而愚蠢（KISS原则）。

评论

从对称的角度来看，let pages = Identity（pagesList）与parseFoo（foo）有何不同？鉴于此，我可能会... {Identity（pagesList），parseFoo（foo），parseBar（bar）}。flatMap（x-> x）。

– ArT
17年6月6日在9:29
感谢您的解释，用三个嵌套的lambda表达式来收集映射列表（对我未经训练的人来说）可能有点太聪明了。

–索比昂·拉文·安德森（ThorbjørnRavn Andersen）
17年6月6日在20:07
评论不作进一步讨论；此对话已移至聊天。

– yannis
17年6月8日在8:03
也许流利的风格在您的第二个示例中会很好？

–user1068
17年6月9日在20:30

#2 楼

如果您有疑问，它可能太聪明了！第二个示例使用诸如foo ? parseFoo(foo) : x => x之类的表达式引入了意外的复杂性，并且总体而言，代码更加复杂，这意味着难以遵循。

所谓的好处是，您可以单独测试各个块，而只需分解成单个功能，就可以通过一种更简单的方式来实现。在第二个示例中，您将原本分开的迭代耦合在一起，因此实际上减少了隔离。

无论您对功能样式的总体感觉如何，这显然都是使代码更复杂的示例。

我发现一些警告信号是，您将简单明了的代码与“新手开发人员”相关联。这是一种危险的心态。以我的经验是相反的：新手开发人员倾向于过于复杂和聪明的代码，因为它需要更多的经验才能看到最简单，最清晰的解决方案。

针对“聪明的代码”的建议并不是关于代码是否使用了新手可能不理解的高级概念。而是要编写比必要的代码更为复杂或复杂的代码。这使得每个人（无论是新手还是专家）都很难遵循该代码，甚至可能几个月后也很难遵循。

评论

“新手开发人员倾向于使用过于复杂和聪明的代码，因为它需要更多的经验才能看到最简单，最清晰的解决方案。”优秀的答案！

– Bonifacio
17年6月6日在11:16
过于复杂的代码也非常被动。您正在故意生成很少有其他人可以轻松阅读或调试的代码...这对您而言意味着工作安全，而在您不在的情况下对其他所有人完全不利。您也可以完全用拉丁语写技术文档。

–伊凡
17年6月6日在20:39
我不认为聪明的代码总是炫耀的东西。有时感觉很自然，并且仅在第二次检查时看起来很荒谬。

–user69767
17年7月7日在7:43
我删除了关于“炫耀”的措辞，因为它听起来比预期的要更具判断力。

–雅克B
17年7月7日在9:17
@BaileyS-我认为这强调了代码审查的重要性；对于编码人员来说，自然而直接的感觉，特别是当以这种方式逐渐发展时，对于审阅者来说似乎很容易被弄乱。然后，代码必须经过重构/重写以消除卷积，然后才能通过审核。

–迈尔斯
17年6月8日在16:15

#3 楼

我的答案来得有点晚，但是我仍然想插话。仅仅因为您使用的是最新的ES6技术或使用最受欢迎的编程范例，并不一定意味着您的代码更正确，或者您的初级代码是错的。实际需要时，应使用函数式编程（或任何其他技术）。如果您试图找到将最新的编程技术塞入每个问题的最佳机会，那么您总是会得到过度设计的解决方案。

退后一步，尝试说出您要解决的问题。本质上，您只希望函数addPages将apiData的不同部分转换为一组键值对，然后将它们全部添加到PagesList中。

如果仅此而已，为什么还要麻烦地使用identity function和ternary operator或使用functor进行输入解析？此外，为什么您甚至认为应用functional programming会引起副作用（通过更改列表）是一种适当的方法？为什么所有这些事情，只要您需要：

 const processFooPages = (foo) => foo ? [['foo', foo]] : [];
const processBarPages = (bar) => bar ? bar.map(page => [page.name, page.data]) : [];
const processBazPages = (baz) => baz ? baz.map(page => [page.id, page.content]) : [];

const addPages = (apiData) => {
  const list = new PagesList();
  const pages = [].concat(
    processFooPages(apiData.pages.foo),
    processBarPages(apiData.pages.arrayOfBars),
    processBazPages(apiData.pages.customBazes)
  );
  pages.forEach(([pageName, pageContent]) => list.addPage(pageName, pageContent));

  return list;
}


（可运行jsfiddle此处）

如您所见，这种方法仍然使用functional programming，但要适度。还要注意，由于所有3个转换函数都不会引起任何副作用，因此它们很容易测试。 addPages中的代码也是微不足道的，并且假定新手或专家只需一眼就能理解。

现在，将此代码与上面的代码进行比较，您看到区别了吗？毫无疑问，functional programming和ES6语法功能强大，但是，如果使用这些技术以错误的方式对问题进行分切，最终将得到更加混乱的代码。

如果您不急于解决问题，并在正确的位置应用正确的技术，则可以拥有在自然界中起作用的代码，而所有团队成员仍然可以很好地组织和维护它们。这些特征不是互斥的。

评论

+1指出这种广泛的态度（不一定适用于OP）：“仅仅因为您使用的是最新的ES6技术或使用最流行的编程范例，并不一定意味着您的代码更正确，或那个初中的代码是错误的。”。

–乔治
17年10月10日在8:24
+1。只是一小段花言巧语，您可以使用spread（...）运算符而不是_.concat来删除该依赖项。

– YoTengoUnLCD
17年6月11日在5:10
@YoTengoUnLCD啊，很好。现在您知道我和我的团队仍在不了解我们的lodash用法的旅途中。该代码可以使用扩展运算符，或者如果想要保持完整的代码形状，甚至可以使用[] .concat（）。

–b0nyb0y
17年6月11日在9:15
抱歉，但是此代码清单对我而言仍然不如OP帖子中的原始“初级代码”明显。基本上：如果可以避免，请勿使用三元运算符。太紧张了在“实际”功能语言中，if语句将是表达式而不是语句，因此更具可读性。

– OlleHärstedt
17年6月12日在20:15
@OlleHärstedt嗯，这是您提出的一个有趣的主张。问题是，函数式编程范式或那里的任何范式永远都不会与任何特定的“真实”功能语言联系在一起，更不用说其语法了。因此，规定应该或不应该使用什么条件构造只是没有任何意义。无论您是否喜欢，三元运算符都与常规if语句一样有效。 if-else和？：camp之间的可读性争论永无止境，所以我不会介入。我要说的是，用训练有素的眼睛，像这样的线条很难“太紧”。

–b0nyb0y
17年6月13日在16:51

#4 楼

第二个摘要比第一个摘要更具可测试性。为两个代码片段之一设置所有所需的测试将相当简单。

这两个摘要之间的真正区别在于可理解性。我可以相当快地阅读第一个代码片段，并了解发生了什么。第二个片段，不是很多。它远不那么直观，而且更长。

这使得第一个代码片段易于维护，这是有价值的代码质量。在第二个片段中，我发现几乎没有任何价值。

#5 楼

TD; DR


您可以在10分钟或更短的时间内向Junior Developer解释您的代码吗？
从现在开始的两个月后，您可以理解您的代码了吗？

详细的分析

清晰度和可读性

对于任何级别的程序员而言，原始代码都非常清晰易懂。它是每个人都熟悉的样式。

可读性主要基于熟悉程度，而不是令牌的一些数学计算。 IMO，在此阶段，您的ES6重写过多。也许几年后，我会更改答案的这一部分。 :-)顺便说一句，我也喜欢@ b0nyb0y答案，这是一个合理而明确的折衷办法。

可测试性

if(apiData.pages.foo){
   pagesList.add('foo', apiData.pages.foo){
}


假定PagesList.add （）具有测试，应该这样做，这是完全简单的代码，并且本节没有明显的理由需要特殊的单独测试。

if (apiData.pages.arrayOfBars){
      let bars = apiData.pages.arrayOfBars;
      bars.forEach((bar) => {
         pagesList.add(bar.name, bar.data);
      })
   }


我再次看到无需立即对此部分进行任何特殊的单独测试。除非PagesList.add（）出现不正常的问题，包括null或重复项或其他输入。 

if (apiData.pages.customBazes) {
      let bazes = apiData.pages.customBazes;
      bazes.forEach((baz) => {
         pagesList.add(customBazParser(baz)); 
      })
   }


这段代码也非常简单。假设customBazParser已经过测试，并且没有返回太多“特殊”结果。再说一次，除非`PagesList.add（）遇到棘手的情况（可能是因为我不熟悉您的域），否则我不明白为什么本节需要进行特殊测试。

通常，测试整个功能应该可以正常工作。

免责声明：如果出于特殊原因要测试三个if()语句的所有8种可能性，那么可以，请拆分测试。或者，如果PagesList.add()敏感，可以，将测试拆分。

结构：是否值得分为三个部分（例如高卢）

在这里，您有最好的说法。就个人而言，我认为原始代码不会“太长”（我不是SRP的狂热者）。但是，如果还有更多部分，则SRP抬起头来就很丑陋，值得进行拆分。尤其是如果应用了DRY并且功能可以在代码的其他地方使用。

我的一个建议

YMMV。为了节省一行代码和一些逻辑，我可以将if和let合并为一行：例如，如果apiData.pages.arrayOfBars是一个数字或字符串，但是原始代码也是如此。对我来说，这更清楚（也是一个习惯用法）。