私は上級フロントエンド開発者で、Babel ES6でコーディングしています。 私たちのアプリの一部がAPIコールを行い、APIコールから戻ってきたデータモデルに基づいて、特定のフォームを入力する必要があります。
それらのフォームは、二重にリンクされたリストに格納されています(バックエンドがデータの一部が無効であると言った場合、リストを修正するだけで、ユーザーを混乱した1つのページに素早く戻し、目標に戻すことができます)。
とにかく、ページを追加するためにたくさんの関数が使われていますが、私はあまり賢くないのではないかと思っています。 これは基本的な概要です。実際のアルゴリズムはもっと複雑で、たくさんの異なるページやページタイプがありますが、これは一例に過ぎません。
これは、初心者のプログラマーが扱う方法だと思います。
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 functorやcurrying、ternary statementなどの概念に慣れていない若手開発者としてこの記事を読んだとしたら、後者のソリューションが何をしているのかを理解することができるでしょうか? 時には、間違った、より簡単な方法で物事を行う方が良いのでしょうか?
あなたのコードでは、複数の変更を行っています。
ここで最も混乱する部分の一つは、関数型プログラミングと命令型プログラミングをどのように混ぜ合わせているかということです。ファンクタを使ってデータを変換しているのではなく、mutable list を様々な関数に渡すために使っているのです。これはあまり有用な抽象化とは思えません。抽象化されるべきだったもの、つまり、その項目が存在する場合にのみ解析するというものは、あなたのコードの中に明示的に存在していますが、今度は隅々まで考えなければなりません。たとえば、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リストの変更性は、複数の呼び出しに分散するのではなく、1つの関数にまとめられました。非局所的な変異を避けることは、
Monad`のようなおかしな名前の抽象化よりも、関数型プログラミングのはるかに大きな部分です。
そう、あなたのコードはとても賢いのです。あなたの賢さは、賢いコードを書くためではなく、あからさまな賢さを必要としないような賢い方法を見つけるために使ってください。最高のデザインとは、派手に見えるものではなく、誰が見ても明らかなものです。また、優れた抽象化は、プログラミングを単純化するためにあり、自分の頭の中で最初に解き明かさなければならない余分なレイヤーを追加するためにあるのではありません(ここでは、ファンクタが変数と同等であり、効果的に省略できることを理解すること)。
お願い:疑わしい場合は、コードをシンプルかつバカにしましょう(KISS原則)。
疑わしいと思ったら、それはおそらく賢すぎるのです。2つ目の例では、foo ? parseFoo(foo) : x => x
のような式で*偶然に複雑さを増しており、全体的にコードが複雑になっているため、従うのが難しくなっています。
全体的にコードが複雑になってしまい、理解するのが難しくなってしまいます。また、2つ目の例では、他の方法では別々に行われる反復を結合しているので、実際には分離は少ないです。
一般的な関数型スタイルに対する考えがどうであれ、これは明らかにコードをより複雑にしている例です。
私は、あなたがシンプルでわかりやすいコードを「初心者の開発者」と結びつけていることに、ちょっとした警告信号を感じています。これは危険なメンタリティだと思います。私の経験では、その逆です。初心者の開発者は、過度に複雑で巧妙なコードになりがちなのです。
巧妙なコードを避けるためのアドバイスは、そのコードが初心者には理解できないような高度な概念を使っているかどうかということではありません。むしろ、必要以上に複雑なコードを書くことが問題なのです。このようなコードは、初心者も熟練者も含めたすべての人*にとって、そしておそらく数ヶ月後のあなた自身にとっても、コードを理解するのが難しくなります。
この回答は少し遅かったのですが、それでも私はコメントしたいと思います。あなたが最新のES6技術を使っているからといって、あなたのコードがより正しく、ジュニアのコードが間違っているとは限りません。機能的プログラミング(あるいはその他の技術)は、実際に必要とされるときに使うべきです。すべての問題に最新のプログラミング技術を詰め込むためのわずかなチャンスを見つけようとすると、必ず過剰に設計されたソリューションになってしまいます。
一歩下がって、あなたが解決しようとしている問題を少しだけ言語化してみてください。要するに、あなたは apiData
のさまざまな部分をキーと値のペアのセットに変換し、それらをすべて PagesList
に追加する関数 addPages
が欲しいだけなのです。
もしそれだけなら、なぜわざわざidentity function
をternary operator
と一緒に使ったり、functor
を使って入力を解析したりするのでしょうか?それに、(リストを変更することで)副作用を引き起こす「関数型プログラミング」を適用することが、なぜ適切なアプローチだと思うのでしょうか?必要なのはこれだけなのに、なぜそんなことをするのですか?
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 [ここ][1])
ご覧のように、このアプローチでは、「関数型プログラミング」を適度に使用しています。また、3つの変換関数は副作用が一切ないので、テストが非常に簡単であることにも注目してください。また、addPages
のコードは、初心者でも上級者でも一目見ただけで理解できるような、些細で控えめなものになっています。
さて、このコードを上で思いついたものと比較してみてください。確かに「関数型プログラミング」や「ES6構文」は強力ですが、これらのテクニックで問題の切り方を間違えると、さらに厄介なコードになってしまいます。
問題に直面しても焦らず、適切な技術を適切な場所に適用すれば、機能的な性質を持ちながら、チームメンバー全員にとって非常に整理されたメンテナンス性の高いコードを作成することができます。これらの特性は相互に排他的なものではありません。