Apakah saya juga 'pintar' untuk dibaca oleh Jr. devs? Terlalu banyak pemrograman fungsional di JS saya? [Tutup]

Saya seorang Sr front-end dev, coding di Babel ES6. Bagian dari aplikasi kami melakukan panggilan API, dan berdasarkan model data yang kami dapatkan dari panggilan API, formulir tertentu perlu diisi.

Formulir-formulir itu disimpan dalam daftar yang ditautkan dua kali lipat (jika back-end mengatakan beberapa data tidak valid, kami dapat dengan cepat mendapatkan pengguna kembali ke satu halaman yang mereka buat berantakan dan kemudian membuatnya kembali sesuai target, hanya dengan memodifikasi) daftar.)

Pokoknya, ada banyak fungsi yang digunakan untuk menambahkan halaman, dan saya bertanya-tanya apakah saya terlalu pintar. Ini hanya gambaran umum dasar - algoritma yang sebenarnya jauh lebih kompleks, dengan banyak halaman dan tipe halaman yang berbeda, tetapi ini akan memberi Anda contoh.

Ini adalah bagaimana, saya pikir, seorang programmer pemula akan menanganinya.

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;
}

Sekarang, agar lebih dapat diuji, saya telah mengambil semua pernyataan if dan membuatnya terpisah, berdiri sendiri fungsinya, dan kemudian saya memetakannya.

Sekarang, dapat diuji adalah satu hal, tetapi juga dapat dibaca dan saya bertanya-tanya apakah saya membuat hal-hal yang kurang dapat dibaca di sini.

// 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

}

Inilah kekhawatiran saya. Bagi saya bagian bawah lebih teratur. Kode itu sendiri dipecah menjadi potongan-potongan kecil yang dapat diuji secara terpisah. TETAPI saya berpikir: Jika saya harus membacanya sebagai pengembang junior, tidak terbiasa dengan konsep-konsep seperti menggunakan fungsi Identitas, currying, atau pernyataan ternary, apakah saya dapat memahami apa yang dilakukan solusi terakhir? Apakah lebih baik melakukan sesuatu dengan cara yang "salah, lebih mudah" kadang-kadang?

Dalam kode Anda, Anda telah membuat banyak perubahan:

• merusak penugasan ke bidang akses di pagesadalah perubahan yang baik.
• mengekstraksi parseFoo()fungsi dll adalah kemungkinan perubahan yang baik.
• memperkenalkan functor adalah ... sangat membingungkan.

Salah satu bagian yang paling membingungkan di sini adalah bagaimana Anda mencampur pemrograman fungsional dan imperatif. Dengan functor Anda, Anda tidak benar-benar mengubah data, Anda menggunakannya untuk meneruskan daftar yang dapat diubah melalui berbagai fungsi. Itu tidak tampak seperti abstraksi yang sangat berguna, kami sudah memiliki variabel untuk itu. Hal yang seharusnya telah diabstraksi - hanya mengurai item itu jika ada - masih ada dalam kode Anda secara eksplisit, tetapi sekarang kita harus berpikir di tikungan. Misalnya, agak tidak jelas parseFoo(foo)akan mengembalikan fungsi. JavaScript tidak memiliki sistem tipe statis untuk memberi tahu Anda apakah ini legal, jadi kode tersebut benar-benar rentan kesalahan tanpa nama yang lebih baik ( makeFooParser(foo)?). Saya tidak melihat manfaat apa pun dalam kebingungan ini.

Apa yang saya harapkan untuk melihat:

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

Tapi itu juga tidak ideal, karena tidak jelas dari situs panggilan bahwa item akan ditambahkan ke daftar halaman. Jika alih-alih fungsi parsing murni dan mengembalikan daftar (mungkin kosong) yang bisa kita tambahkan ke halaman secara eksplisit, itu mungkin lebih baik:

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

Manfaat tambahan: logika tentang apa yang harus dilakukan ketika item kosong sekarang telah dipindahkan ke fungsi parsing individual. Jika ini tidak tepat, Anda masih bisa memperkenalkan persyaratan. Mutabilitas pagesdaftar sekarang ditarik bersama menjadi satu fungsi, alih-alih menyebarkannya di beberapa panggilan. Menghindari mutasi non-lokal adalah bagian yang jauh lebih besar dari pemrograman fungsional daripada abstraksi dengan nama-nama lucu seperti Monad.

Jadi ya, kode Anda terlalu pintar. Harap terapkan kepintaran Anda untuk tidak menulis kode pintar, tetapi untuk menemukan cara pintar untuk menghindari perlunya kepintaran yang terang-terangan. Desain terbaik tidak terlihat mewah, tetapi terlihat jelas bagi siapa saja yang melihatnya. Dan abstraksi yang baik ada untuk menyederhanakan pemrograman, bukan untuk menambahkan lapisan tambahan yang harus saya uraikan dalam pikiran saya terlebih dahulu (di sini, mencari tahu bahwa functor setara dengan variabel, dan secara efektif dapat dihindari).

Harap: jika ragu, simpan kode Anda sederhana dan bodoh (prinsip KISS).

Jika Anda ragu, mungkin itu terlalu pintar! Contoh kedua memperkenalkan kompleksitas yang tidak disengaja dengan ekspresi seperti foo ? parseFoo(foo) : x => x, dan secara keseluruhan kode lebih kompleks yang berarti lebih sulit untuk diikuti.

Manfaat yang diakui, bahwa Anda dapat menguji potongan secara individual, dapat dicapai dengan cara yang lebih sederhana dengan hanya membobol fungsi individu. Dan dalam contoh kedua Anda memasangkan iterasi yang terpisah, sehingga Anda benar-benar mendapatkan lebih sedikit isolasi.

Apa pun perasaan Anda tentang gaya fungsional secara umum, ini jelas merupakan contoh di mana itu membuat kode lebih kompleks.

Saya menemukan sedikit sinyal peringatan bahwa Anda mengaitkan kode sederhana dan langsung dengan "pengembang pemula". Ini adalah mentalitas yang berbahaya. Dalam pengalaman saya, yang terjadi adalah sebaliknya: Pengembang pemula cenderung terhadap kode yang terlalu rumit dan pintar, karena membutuhkan lebih banyak pengalaman untuk dapat melihat solusi paling sederhana dan paling jelas.

Saran terhadap "kode pintar" tidak benar-benar tentang apakah kode tersebut menggunakan konsep-konsep lanjutan yang mungkin tidak dipahami oleh seorang pemula. Melainkan soal menulis kode yang lebih rumit atau berbelit-belit dari yang diperlukan . Ini membuat kode lebih sulit untuk diikuti untuk semua orang , baik pemula maupun pakar, dan mungkin juga untuk Anda sendiri beberapa bulan ke depan.

Jawaban saya ini datang agak terlambat, tapi saya masih ingin berpadu. Hanya karena Anda menggunakan teknik ES6 terbaru atau menggunakan paradigma pemrograman paling populer tidak selalu berarti bahwa kode Anda lebih benar, atau kode junior itu salah. Pemrograman Fungsional (atau teknik lainnya) harus digunakan ketika itu benar-benar diperlukan. Jika Anda mencoba menemukan peluang terkecil untuk menjejalkan teknik pemrograman terbaru ke dalam setiap masalah, Anda akan selalu berakhir dengan solusi rekayasa berlebihan.

Ambil langkah mundur, dan cobalah untuk mengungkapkan masalah yang Anda coba selesaikan sebentar. Intinya, Anda hanya ingin fungsi addPagesmengubah bagian yang berbeda apiDatamenjadi satu set pasangan nilai kunci, lalu menambahkan semuanya PagesList.

Dan kalau itu semua ada untuk itu, mengapa repot-repot menggunakan identity functiondengan ternary operator, atau menggunakan functoruntuk input parsing? Selain itu, mengapa Anda berpikir itu adalah pendekatan yang tepat untuk diterapkan functional programmingyang menyebabkan efek samping (dengan memutasikan daftar)? Mengapa semua itu, padahal yang Anda butuhkan hanyalah ini:

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 runnable di sini )

Seperti yang Anda lihat, pendekatan ini masih digunakan functional programming, tetapi tidak berlebihan. Juga perhatikan bahwa karena ketiga fungsi transformasi tidak menimbulkan efek samping apa pun, mereka mudah untuk diuji. Kode dalam addPagesjuga sepele dan sederhana yang pemula atau ahli dapat mengerti hanya dengan pandangan sekilas.

Sekarang, bandingkan kode ini dengan yang Anda buat di atas, apakah Anda melihat perbedaannya? Tidak diragukan lagi, functional programmingdan sintaksis ES6 kuat, tetapi jika Anda mengiris masalahnya dengan cara yang salah dengan teknik ini, Anda akan berakhir dengan kode bahkan lebih berantakan.

Jika Anda tidak tergesa-gesa dalam masalah, dan menerapkan teknik yang tepat di tempat yang tepat, Anda dapat memiliki kode yang sifatnya fungsional, sementara masih sangat terorganisir dan dikelola oleh semua anggota tim. Karakteristik ini tidak saling eksklusif.

Cuplikan kedua tidak lebih dapat diuji dari yang pertama. Cukup mudah untuk mengatur semua tes yang diperlukan untuk salah satu dari dua cuplikan.

Perbedaan nyata antara kedua cuplikan adalah kelengkapan. Saya dapat membaca cuplikan pertama dengan cukup cepat dan memahami apa yang terjadi. Cuplikan kedua, tidak terlalu banyak. Ini jauh kurang intuitif, serta jauh lebih lama.

Itu membuat cuplikan pertama lebih mudah dipelihara, yang merupakan kualitas kode yang berharga. Saya menemukan sangat sedikit nilai di cuplikan kedua.

TD; DR

1. Bisakah Anda menjelaskan kode Anda ke Pengembang Junior dalam 10 menit atau kurang?
2. Dua bulan dari sekarang, bisakah Anda memahami kode Anda?

Analisis terperinci

Kejelasan dan Keterbacaan

Kode asli sangat jelas dan mudah dimengerti untuk semua level programmer. Ini adalah gaya yang akrab bagi semua orang .

Keterbacaan sebagian besar didasarkan pada keakraban, bukan penghitungan token matematis . IMO, pada tahap ini, Anda memiliki terlalu banyak ES6 dalam penulisan ulang. Mungkin dalam beberapa tahun saya akan mengubah bagian jawaban saya ini. :-) BTW, saya juga suka jawaban @ b0nyb0y sebagai kompromi yang masuk akal dan jelas.

Testabilitas

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

Dengan asumsi bahwa PagesList.add () memiliki tes, yang seharusnya, ini adalah kode yang sangat mudah dan tidak ada alasan yang jelas untuk bagian ini membutuhkan pengujian terpisah yang khusus.

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

Sekali lagi, saya tidak melihat kebutuhan mendesak untuk pengujian terpisah khusus pada bagian ini. Kecuali PagesList.add () memiliki masalah tidak biasa dengan nol atau duplikat atau input lainnya.

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

Kode ini juga sangat mudah. Dengan asumsi yang customBazParserdiuji dan tidak mengembalikan terlalu banyak hasil "khusus". Jadi sekali lagi, kecuali ada situasi rumit dengan `PagesList.add (), (yang mungkin ada karena saya tidak terbiasa dengan domain Anda) Saya tidak melihat mengapa bagian ini membutuhkan pengujian khusus.

Secara umum, pengujian seluruh fungsi harus bekerja dengan baik.

Penafian : Jika ada alasan khusus untuk menguji semua 8 kemungkinan dari ketiga if()pernyataan, maka ya, lakukan split test. Atau, jika PagesList.add()sensitif, ya, bagi tes.

Struktur: Apakah layak dipecah menjadi tiga bagian (seperti Galia)

Di sini Anda memiliki argumen terbaik. Secara pribadi, saya tidak berpikir kode asli "terlalu panjang" (Saya bukan penggemar SRP). Tapi, jika ada beberapa if (apiData.pages.blah)bagian lagi , maka SRP memunculkannya yang jelek dan akan layak untuk dibelah. Terutama jika KERING berlaku dan fungsi dapat digunakan di tempat lain dari kode.

Satu saran saya

YMMV. Untuk menyimpan satu baris kode dan beberapa logika, saya mungkin menggabungkan if dan membiarkannya menjadi satu baris: mis

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

Ini akan gagal jika apiData.pages.arrayOfBars adalah Number atau String, tetapi demikian juga kode aslinya. Dan bagi saya itu lebih jelas (dan idiom yang terlalu sering digunakan).