Pada titik apakah keringkasan tidak lagi menjadi kebajikan?

102

Perbaikan bug baru-baru ini mengharuskan saya membaca kode yang ditulis oleh anggota tim lain, tempat saya menemukan ini (ini C #):

return (decimal)CostIn > 0 && CostOut > 0 ? (((decimal)CostOut - (decimal)CostIn) / (decimal)CostOut) * 100 : 0;

Sekarang, membiarkan ada alasan yang bagus untuk semua pemain itu, ini sepertinya masih sangat sulit untuk diikuti. Ada bug kecil dalam perhitungan dan saya harus menguraikannya untuk memperbaiki masalah.

Saya tahu gaya pengkodean orang ini dari ulasan kode, dan pendekatannya adalah bahwa lebih pendek hampir selalu lebih baik. Dan tentu saja ada nilai di sana: kita semua telah melihat rantai logika kondisional kompleks yang tidak perlu yang dapat dirapikan dengan beberapa operator yang ditempatkan dengan baik. Tapi dia jelas lebih mahir daripada saya dalam mengikuti rantai operator yang dijejalkan ke dalam satu pernyataan.

Ini, tentu saja, pada akhirnya adalah masalah gaya. Tetapi apakah ada sesuatu yang ditulis atau diteliti tentang mengenali titik di mana perjuangan untuk kode singkat berhenti menjadi berguna dan menjadi penghalang bagi pemahaman?

Alasan para pemain adalah Entity Framework. Db perlu menyimpan ini sebagai tipe yang dapat dibatalkan. Desimal? tidak setara dengan Desimal dalam C # dan perlu dicetak.

c# coding-style code-quality clean-code Bob Tway
sumber
153
Saat singkat truf mudah dibaca.
Robert Harvey
27
Melihat contoh spesifik Anda: gips adalah (1) tempat di mana pengembang tahu lebih dari kompiler dan perlu memberi tahu kompiler fakta yang tidak dapat disimpulkan, atau (2) di mana beberapa data disimpan di "salah" "ketik untuk jenis operasi yang perlu kita lakukan di atasnya. Keduanya merupakan indikator kuat bahwa sesuatu dapat di refactored. Solusi terbaik di sini adalah menemukan cara untuk menulis kode tanpa gips.
Eric Lippert
29
Secara khusus, tampaknya aneh bahwa perlu untuk memasukkan CostIn ke desimal untuk membandingkannya dengan nol, tetapi bukan CostOut; mengapa demikian? Apa yang ada di bumi adalah jenis CostIn yang hanya dapat dibandingkan dengan nol dengan melemparkannya ke desimal? Dan mengapa CostOut tidak dari jenis yang sama dengan CostIn?
Eric Lippert
12
Lagipula, logika di sini mungkin sebenarnya salah. Misalkan CostOutsama dengan Double.Epsilon, dan karena itu lebih besar dari nol. Tetapi (decimal)CostOutdalam hal ini nol, dan kami memiliki pembagian dengan kesalahan nol. Langkah pertama harus mendapatkan kode yang benar , yang saya pikir tidak. Dapatkan benar, buat test case, dan kemudian buat elegan . Kode elegan dan kode singkat memiliki banyak kesamaan, tetapi terkadang singkatnya bukanlah jiwa keanggunan.
Eric Lippert
5
Keringkasan selalu merupakan kebajikan. Tetapi fungsi obyektif kami menggabungkan singkatnya dengan kebajikan lainnya. Jika seseorang bisa lebih singkat tanpa merusak kebajikan lainnya, ia harus selalu demikian.
Rahasia Solomonoff

Jawaban:

163

Untuk menjawab pertanyaan Anda tentang penelitian yang masih ada

Tetapi apakah ada sesuatu yang ditulis atau diteliti tentang mengenali titik di mana perjuangan untuk kode singkat berhenti menjadi berguna dan menjadi penghalang bagi pemahaman?

Ya, ada pekerjaan di bidang ini.

Untuk mendapatkan pemahaman tentang hal ini, Anda harus menemukan cara untuk menghitung metrik sehingga perbandingan dapat dilakukan secara kuantitatif (bukan hanya melakukan perbandingan berdasarkan kecerdasan dan intuisi, seperti jawaban lainnya). Satu metrik potensial yang telah dilihat adalah

Kompleksitas Siklomatik ÷ Source Lines of Code ( SLOC )

Dalam contoh kode Anda, rasio ini sangat tinggi, karena semuanya telah dikompresi menjadi satu baris.

SATC telah menemukan evaluasi yang paling efektif adalah kombinasi dari ukuran dan kompleksitas [Siklomatik]. Modul dengan kompleksitas tinggi dan ukuran besar cenderung memiliki keandalan terendah. Modul dengan ukuran rendah dan kompleksitas tinggi juga merupakan risiko keandalan karena mereka cenderung kode yang sangat singkat, yang sulit untuk diubah atau dimodifikasi.

Tautan

Berikut adalah beberapa referensi jika Anda tertarik:

McCabe, T. dan A. Watson (1994), Kompleksitas Perangkat Lunak (CrossTalk: Jurnal Rekayasa Perangkat Lunak Pertahanan).

Watson, AH, & McCabe, TJ (1996). Structured Testing: Suatu Metodologi Pengujian Menggunakan Metrik Kompleksitas Siklomatik (Publikasi Khusus NIST 500-235). Diperoleh 14 Mei 2011, dari situs web Perangkat Lunak McCabe: http://www.mccabe.com/pdf/mccabe-nist235r.pdf

Rosenberg, L., Hammer, T., Shaw, J. (1998). Metrik dan Keandalan Perangkat Lunak (Prosiding Simposium Internasional IEEE tentang Rekayasa Keandalan Perangkat Lunak). Diperoleh 14 Mei 2011, dari situs web Universitas Negeri Penn: http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.104.4041&rep=rep1&type=pdf

Pendapat dan solusi saya

Secara pribadi, saya tidak pernah menghargai singkatnya, hanya keterbacaan. Terkadang singkat membantu kesiapan, kadang tidak. Yang lebih penting adalah Anda menulis Really Obvious Code (ROC) alih-alih Write-Only Code (WOC).

Hanya untuk bersenang-senang, inilah cara saya akan menulisnya, dan meminta anggota tim saya untuk menulisnya:

if ((costIn <= 0) || (costOut <= 0)) return 0;
decimal changeAmount = costOut - costIn;
decimal changePercent = changeAmount / costOut * 100;
return changePercent;

Perhatikan juga pengenalan variabel kerja memiliki efek samping yang menyenangkan dari memicu aritmatika titik tetap dan bukan aritmatika bilangan bulat, sehingga kebutuhan semua gips untuk decimaldihilangkan.

John Wu
sumber
4
Saya sangat suka klausa penjaga untuk kasus kurang dari nol. Mungkin layak komentar: bagaimana biayanya bisa kurang dari nol, kasus khusus apa itu?
user949300
22
+1. Saya mungkin akan menambahkan sesuatu sepertiif ((costIn < 0) || (costOut < 0)) throw new Exception("costs must not be negative");
Doc Brown
13
@DocBrown: Itu saran yang bagus, tapi saya akan mempertimbangkan apakah jalur kode yang luar biasa dapat dilakukan dengan tes. Jika ya, maka tulis uji kasus yang menggunakan jalur kode itu. Jika tidak, maka ubahlah semuanya menjadi sebuah Pernyataan.
Eric Lippert
12
Meskipun ini semua adalah poin yang baik, saya percaya ruang lingkup pertanyaan ini adalah, gaya kode, bukan logika. Snip kode saya adalah upaya kesetaraan fungsional dari perspektif kotak hitam. Melempar pengecualian tidak sama dengan mengembalikan 0, sehingga solusi tidak akan setara secara fungsional.
John Wu
30
"Secara pribadi, saya tidak pernah menghargai keringkasan, hanya keterbacaan. Kadang keringkasan membantu kesiapan, kadang tidak." Baik titik. +1 hanya untuk itu. Saya suka solusi kode Anda juga.
Wildcard
49

Brevity baik ketika mengurangi kekacauan di sekitar hal-hal yang penting, tetapi ketika menjadi singkat , mengemas terlalu banyak data yang relevan terlalu padat untuk diikuti, maka data yang relevan menjadi berantakan sendiri dan Anda memiliki masalah.

Dalam kasus khusus ini, gips untuk decimaldiulangi berulang kali; secara keseluruhan mungkin akan lebih baik untuk menulis ulang sebagai sesuatu seperti:

var decIn = (decimal)CostIn;
var decOut = (decimal)CostOut;
return decIn > 0 && CostOut > 0 ? (decOut - decIn ) / decOut * 100 : 0;
//                  ^ as in the question

Tiba-tiba garis yang berisi logika jauh lebih pendek dan pas pada satu garis horizontal, sehingga Anda dapat melihat semuanya tanpa harus menggulir, dan maknanya jauh lebih mudah terlihat.

Mason Wheeler
sumber
1
Saya mungkin akan pergi lebih jauh dan refactored ((decOut - decIn ) / decOut) * 100ke variabel lain.
FrustratedWithFormsDesigner
9
Assembler jauh lebih jelas: hanya satu operasi per baris. Doh!
2
@FrustratedWithFormsDesigner Saya bahkan akan melangkah lebih jauh dan membungkus cek bersyarat dalam tanda kurung.
Chris Cirefice
1
@FrustratedWithFormsDesigner: Mengekstrak ini sebelum kondisional akan melewati cek divison-by-zero ( CostOut > 0), jadi Anda harus memperluas kondisional ke dalam- ifpernyataan. Bukannya ada yang salah dengan ini, tapi itu menambah lebih banyak kata daripada sekadar pengenalan lokal.
wchargin
1
Jujur saja, hanya menghilangkan gips sepertinya Anda melakukan cukup udara IMO, jika seseorang sulit membaca karena dia tidak bisa mengenali perhitungan tarif dasar sederhana, itu masalahnya, bukan milikku.
Walfrat
7

Meskipun saya tidak dapat mengutip penelitian khusus apa pun tentang masalah ini, saya akan menyarankan agar semua pemeran dalam kode Anda melanggar prinsip Don't Repeat Yourself. Apa yang tampaknya dilakukan oleh kode Anda adalah mengonversi costIndan costOutmengetik Decimal, dan kemudian melakukan beberapa pemeriksaan kewarasan atas hasil konversi tersebut, dan melakukan operasi tambahan pada nilai-nilai yang dikonversi jika pemeriksaan lulus. Bahkan, kode Anda melakukan salah satu pemeriksaan kewarasan pada nilai yang belum dikonversi, meningkatkan kemungkinan bahwa costOut mungkin memiliki nilai yang lebih besar dari nol, tetapi kurang dari setengah ukuran dari bukan nol terkecil yang Decimaldapat mewakili. Kode akan jauh lebih jelas jika mendefinisikan variabel tipe Decimaluntuk menyimpan nilai yang dikonversi, dan kemudian menindaklanjutinya.

Tampaknya penasaran bahwa Anda akan lebih tertarik pada rasio Decimalrepresentasi costIndan costOutdaripada rasio nilai aktual costIndan costOut, kecuali jika kode tersebut juga akan menggunakan representasi desimal untuk beberapa tujuan lain. Jika kode akan menggunakan representasi tersebut lebih lanjut, itu akan menjadi argumen lebih lanjut untuk membuat variabel untuk menahan representasi tersebut, daripada memiliki urutan gips yang berkelanjutan di seluruh kode.

supercat
sumber
Gips (desimal) mungkin untuk memenuhi beberapa persyaratan aturan bisnis. Saat berhadapan dengan akuntan, terkadang Anda harus melewati rintangan bodoh. Saya pikir CFO akan mengalami serangan jantung ketika ia menemukan garis "Gunakan koreksi pembulatan pajak - $ 0,01" yang tidak dapat dihindari mengingat fungsionalitas yang diminta. (Disediakan: Harga setelah pajak. Saya harus menentukan harga sebelum pajak. Peluang tidak akan ada jawaban sama dengan tarif pajak.)
Loren Pechtel
@LorenPechtel: Mengingat bahwa ketepatan yang Decimaldilemparkan oleh para pemain tergantung pada besarnya nilai yang dipermasalahkan, saya merasa sulit untuk membayangkan aturan bisnis yang akan memanggil cara para pemain sebenarnya akan berperilaku.
supercat
1
Poin bagus - Saya sudah lupa detail dari tipe desimal karena saya tidak pernah memiliki kesempatan untuk menginginkan sesuatu seperti itu. Saya sekarang berpikir ini adalah kepatuhan kargo terhadap aturan bisnis - khususnya, uang tidak boleh menjadi poin mengambang.
Loren Pechtel
1
@LorenPechtel: Untuk mengklarifikasi poin terakhir saya: tipe titik tetap dapat menjamin bahwa x + yy akan meluap atau menghasilkan y. The Decimaltipe tidak. Nilai 1.0d / 3.0 akan memiliki lebih banyak digit di sebelah kanan desimal daripada yang dapat dipertahankan saat menggunakan angka yang lebih besar. Jadi menambah dan mengurangi jumlah yang sama akan menyebabkan kehilangan presisi. Tipe titik tetap dapat kehilangan presisi dengan penggandaan atau pembagian fraksional, tetapi tidak dengan penambahan, pengurangan, perkalian, atau pembagian-dengan-sisa (misalnya 1,00 / 7 adalah 0,14 sisa 0,2; 1,00 div 0,15 adalah 6 sisa 0,10).
supercat
1
@ Hulk: Ya, tentu saja. Saya berdebat apakah akan menggunakan x + yy menghasilkan x, x + yx menghasilkan y, atau x + yxy, dan akhirnya mish-moshing dua yang pertama. Yang penting adalah bahwa tipe titik tetap dapat menjamin bahwa banyak urutan operasi tidak akan pernah menyebabkan kesalahan pembulatan yang tidak terdeteksi dengan besaran apa pun . Jika kode menambahkan hal-hal bersama-sama dalam berbagai cara untuk memverifikasi bahwa total cocok (misalnya membandingkan jumlah subtotal baris dengan jumlah subtotal kolom), memiliki hasil yang keluar sama persis jauh lebih baik daripada membiarkannya keluar "tutup".
supercat
5

Saya melihat kode itu dan bertanya "bagaimana biayanya 0 (atau kurang)?". Kasus khusus apa yang ditunjukkan? Kode seharusnya

bool BothCostsAreValidProducts = (CostIn > 0) && (CostOut > 0);
if (!BothCostsAreValidProducts)
  return NO_PROFIT;
else {
   // that calculation here...
}

Saya menduga nama di sini: ubah BothCostsAreValidProductsdan NO_PROFITsepantasnya.

pengguna949300
sumber
Tentu saja, biaya bisa nol (pikirkan hadiah Xmas). Dan pada saat suku bunga negatif, biaya negatif seharusnya tidak terlalu mengejutkan juga dan setidaknya kode harus disiapkan untuk menghadapinya (dan baik itu dengan melemparkan kesalahan)
Hagen von Eitzen
Anda di jalur yang benar.
danny117
Itu konyol. if (CostIn <= 0 || CostOut <= 0)baik-baik saja.
Miles Rout
Jauh lebih mudah dibaca. Nama variabelnya mengerikan (BothCostsAreValid akan lebih baik. Tidak ada apa pun di sini tentang produk). Tetapi bahkan itu tidak menambah apa pun untuk dibaca, karena hanya memeriksa CostIn, CostOut baik-baik saja. Anda akan memperkenalkan variabel tambahan dengan nama yang bermakna jika makna ekspresi yang diuji tidak jelas.
gnasher729
5

Brevity berhenti menjadi suatu kebajikan ketika kita lupa bahwa itu berarti suatu tujuan daripada kebajikan itu sendiri. Kami menyukai keringkasan karena berkorelasi dengan kesederhanaan, dan kami menyukai kesederhanaan karena kode yang lebih sederhana lebih mudah dipahami dan lebih mudah untuk memodifikasi dan mengandung lebih sedikit bug. Pada akhirnya kami ingin kode untuk mencapai tujuan ini:

  1. Memenuhi persyaratan bisnis dengan jumlah pekerjaan paling sedikit

  2. Hindari bug

  3. Izinkan kami melakukan perubahan di masa mendatang yang terus memenuhi 1 dan 2

Inilah tujuannya. Prinsip atau metode desain apa pun (baik itu CIUMAN, YAGNI, TDD, SOLID, bukti, sistem tipe, metaprogramming dinamis, dll.) Hanya berbudi luhur sejauh mereka membantu kami mencapai tujuan ini.

Garis yang dimaksud tampaknya telah meleset dari tujuan akhir. Meskipun pendek, itu tidak sederhana. Ini sebenarnya mengandung redundansi yang tidak perlu dengan mengulangi operasi casting yang sama beberapa kali. Kode berulang meningkatkan kompleksitas dan kemungkinan bug. Memadukan casting dengan perhitungan aktual juga membuat kode sulit untuk diikuti.

Jalur ini memiliki tiga masalah: Pelindung (casing khusus 0), tipe casting dan perhitungan. Setiap masalah cukup sederhana ketika diambil secara terpisah, tetapi karena semuanya telah berbaur dalam ekspresi yang sama, itu menjadi sulit untuk diikuti.

Tidak jelas mengapa CostOuttidak dilemparkan pertama kali digunakan CostInadalah. Mungkin ada alasan yang bagus, tetapi maksudnya tidak jelas (setidaknya bukan tanpa konteks) yang berarti pengelola akan berhati-hati mengubah kode ini karena mungkin ada beberapa asumsi tersembunyi. Dan ini adalah laknat untuk rawatan.

Karena CostIndilemparkan sebelum membandingkan dengan 0 Saya menganggap itu adalah nilai floating point. (Jika itu sebuah int tidak akan ada alasan untuk melemparkan). Tetapi jika CostOutfloat maka kode tersebut mungkin menyembunyikan bug divide-by-zero yang tidak jelas, karena nilai floating point mungkin kecil tetapi tidak nol, tetapi nol ketika dilemparkan ke desimal (setidaknya saya percaya ini akan mungkin terjadi.)

Jadi masalahnya bukan singkatnya atau tidak adanya masalah, masalahnya adalah logika berulang dan perpaduan masalah yang mengarah ke kode yang sulit dipelihara.

Memperkenalkan variabel untuk menahan nilai yang dicor mungkin akan meningkatkan ukuran kode yang dihitung dalam jumlah token, tetapi akan mengurangi kompleksitas, memisahkan masalah, dan meningkatkan kejelasan, yang membawa kita lebih dekat ke tujuan kode yang lebih mudah dipahami dan dipelihara.

JacquesB
sumber
1
Poin penting: Casting CostIn sekali bukan dua kali membuatnya tidak dapat dibaca, karena pembaca tidak tahu apakah ini bug halus dengan perbaikan yang jelas, atau apakah ini dilakukan dengan sengaja. Jelas jika saya tidak bisa mengatakan dengan pasti apa yang dimaksud penulis dengan kalimat, maka itu tidak dapat dibaca. Harus ada dua pemain, atau komentar yang menjelaskan mengapa penggunaan pertama CostIn tidak memerlukan atau tidak harus memiliki pemain.
gnasher729
3

Keringkasan bukanlah suatu kebajikan sama sekali. Keterbacaan adalah kebajikan.

Brevity dapat menjadi alat dalam mencapai kebajikan, atau, seperti dalam contoh Anda, dapat menjadi alat dalam mencapai sesuatu yang berlawanan. Dengan cara ini atau yang lain, hampir tidak ada nilainya sendiri. Aturan bahwa kode harus "sesingkat mungkin" dapat diganti dengan "sesederhana mungkin" juga - mereka semua sama-sama tidak berarti dan berpotensi merusak jika mereka tidak melayani tujuan yang lebih besar.

Selain itu, kode yang Anda posting bahkan tidak mengikuti aturan singkatnya. Telah konstanta dideklarasikan dengan M akhiran, sebagian besar mengerikan (decimal)gips bisa dihindari, karena compiler akan mempromosikan tersisa intuntuk decimal. Saya percaya bahwa orang yang Anda gambarkan hanya menggunakan singkat sebagai alasan. Kemungkinan besar tidak disengaja, tapi tetap saja.

Agent_L
sumber
2

Dalam pengalaman bertahun-tahun, saya menjadi percaya bahwa singkatnya waktu adalah waktu - waktu mendominasi yang lainnya. Itu termasuk waktu kinerja - berapa lama program untuk melakukan pekerjaan - dan waktu pemeliharaan - berapa lama waktu untuk menambahkan fitur atau memperbaiki bug. (Bagaimana Anda menyeimbangkan keduanya tergantung pada seberapa sering kode tersebut dilakukan vs ditingkatkan - ingat bahwa pengoptimalan prematur masih merupakan akar dari semua kejahatan .) Keringkasan kode adalah untuk meningkatkan keringkasan keduanya; kode yang lebih pendek biasanya berjalan lebih cepat, dan biasanya lebih mudah dipahami dan karenanya dipelihara. Jika tidak melakukan keduanya, maka itu adalah net negative.

Dalam kasus yang ditunjukkan di sini, saya pikir singkatnya teks telah disalahartikan sebagai singkatnya jumlah baris, dengan mengorbankan keterbacaan, yang dapat meningkatkan waktu pemeliharaan. (Mungkin juga perlu waktu lebih lama untuk melakukan, tergantung pada bagaimana casting dilakukan, tetapi kecuali jika baris di atas dijalankan jutaan kali, itu mungkin bukan masalah.) Dalam hal ini, gips desimal berulang mengurangi keterbacaan, karena sulit untuk lihat apa perhitungan yang paling penting. Saya akan menulis sebagai berikut:

decimal dIn = (decimal)CostIn;
decimal dOut = (decimal)CostOut;
return dIn > 0 && CostOut > 0 ? ((dOut - dIn) / dOut) * 100 : 0;

(Sunting: ini adalah kode yang sama dengan jawaban yang lain, jadi begitulah.)

Saya seorang penggemar operator ternary ? :, jadi saya akan membiarkan itu masuk

Paul Brinkley
sumber
5
Ternary sulit dibaca, terutama jika ada ekspresi di luar nilai tunggal atau variabel dalam nilai kembali.
Almo
Saya bertanya-tanya apakah itu yang mendorong suara negatif. Kecuali apa yang saya tulis sangat setuju dengan Mason Wheeler, saat ini dengan 10 suara. Dia meninggalkan ternary juga. Saya tidak tahu mengapa begitu banyak orang memiliki masalah dengan ? :- Saya pikir contoh di atas cukup kompak, esp. dibandingkan dengan jika-maka-lain.
Paul Brinkley
1
Benar-benar tidak yakin. Saya tidak menurunkan suara Anda. Saya tidak suka terner karena tidak jelas apa yang ada di kedua sisi :. if-elseberbunyi seperti bahasa Inggris: jangan lewatkan artinya.
Almo
FWIW Saya curiga Anda mendapatkan downvotes karena ini adalah jawaban yang sangat mirip dengan Mason Wheeler, tetapi jawabannya adalah yang pertama.
Bob Tway
1
Kematian bagi operator ternary !! (juga, kematian pada tab, atau spasi dan model tanda kurung & indentasi apa pun kecuali Allman (pada kenyataannya, The Donald (tm) telah menge-tweet bahwa ini akan menjadi tiga undang-undang pertama yang diberlakukan pada tanggal 20)
Mawg
2

Seperti hampir semua jawaban di atas keterbacaan harus selalu menjadi tujuan utama Anda. Namun saya juga berpikir memformat bisa menjadi cara yang lebih efektif untuk mencapai ini daripada menciptakan variabel dan baris baru. 

return ((decimal)CostIn > 0 && CostOut > 0) ?
       100 * ( (decimal)CostOut - (decimal)CostIn ) / (decimal)CostOut:
       0;

Saya sangat setuju dengan argumen kompleksitas siklomatik dalam banyak kasus, namun fungsi Anda tampaknya kecil dan cukup sederhana untuk mengatasi dengan lebih baik dengan kasus uji yang baik. Karena penasaran mengapa harus dilemparkan ke desimal?

backpackcoder
sumber
4
Alasan para pemain adalah Entity Framework. Db perlu menyimpan ini sebagai tipe yang dapat dibatalkan. Dua kali lipat? tidak setara dengan Double di C # dan perlu dilemparkan.
Bob Tway
2
@ MatThrower Maksud Anda decimal, bukan? double! = decimal, ada perbedaan besar.
pinkfloydx33
1
@ pinkfloydx33 ya! Mengetik di ponsel dengan hanya setengah otak terlibat :)
Bob Tway
Saya harus menjelaskan kepada siswa saya bahwa tipe data SQL anehnya berbeda dari jenis yang digunakan dalam bahasa pemrograman. Saya belum bisa menjelaskan kepada mereka mengapa . "Aku tidak tahu!" "Little endian!"
3
Saya tidak menemukan ini dapat dibaca sama sekali.
Almo
1

Bagi saya, sepertinya masalah besar dengan keterbacaan di sini terletak pada kurangnya format.

Saya akan menulis seperti ini:

return (decimal)CostIn > 0 && CostOut > 0 
            ? (((decimal)CostOut - (decimal)CostIn) / (decimal)CostOut) * 100 
            : 0;

Tergantung pada apakah tipe CostIndan CostOuttipe floating-point atau tipe integral, beberapa gips mungkin juga tidak diperlukan. Berbeda dengan floatdan double, nilai-nilai integral secara implisit dipromosikan ke decimal.

Felix Dombek
sumber
Saya menyesal melihat ini diturunkan tanpa penjelasan, tetapi bagi saya tampaknya identik dengan jawaban kode ransel dikurangi beberapa komentarnya, jadi saya kira itu dibenarkan.
PJTraill
@ PJTraill Saya pasti melewatkan itu, itu memang hampir identik. Namun, saya sangat suka memiliki operator di jalur baru, itulah sebabnya saya akan membiarkan versi saya berdiri.
Felix Dombek
Saya setuju tentang operator, seperti yang saya katakan dalam komentar pada jawaban lain - saya belum melihat bahwa Anda telah melakukannya seperti yang saya inginkan.
PJTraill
0

Kode dapat ditulis dengan tergesa-gesa, tetapi kode di atas harus di dunia saya ditulis dengan nama variabel yang jauh lebih baik.

Dan jika saya membaca kodenya dengan benar maka ia mencoba membuat perhitungan grossmargin.

var totalSales = CostOut;
var totalCost = CostIn;
var profit = (decimal)(CostOut - CostIn);
var grossMargin = 0m; //profit expressed as percentage of totalSales

if(profit > 0)
{
    grossMargin = profit/totalSales*100
}
Thomas Koelle
sumber
3
Anda kehilangan pembagian dengan nol mengembalikan nol.
danny117
1
dan itu sebabnya sulit untuk memperbaiki kode orang lain yang dimaksimalkan untuk singkatnya dan mengapa itu baik untuk memiliki komentar tambahan untuk menjelaskan mengapa / bagaimana hal-hal bekerja
Rudolf Olah
0

Saya mengasumsikan CostIn * CostOut adalah bilangan bulat
Ini adalah bagaimana saya akan menuliskannya
M (Uang) adalah desimal

return CostIn > 0 && CostOut > 0 ? 100M * (CostOut - CostIn) / CostOut : 0M;
paparazzo
sumber
1
berharap mereka tidak berpikir negatif: p
Walfrat
2
Apakah angka nol masih ada di sana?
danny117
@ danny117 Ketika singkatnya menghasilkan jawaban yang salah maka sudah jauh.
paparazzo
Tidak ingin memperbarui pertanyaan dan membuatnya aktif. 100M dan 0M memaksa desimal. Saya pikir (CostOut - CostIn) akan dilakukan sebagai bilangan bulat matematika dan kemudian perbedaan dilemparkan ke desimal.
paparazzo
0

Kode ditulis untuk dipahami oleh orang-orang; singkatnya dalam hal ini tidak membeli banyak dan meningkatkan beban pada pengelola. Untuk singkatnya ini, Anda harus mengembangkannya dengan membuat kode lebih mendokumentasikan diri sendiri (nama variabel yang lebih baik) atau menambahkan lebih banyak komentar yang menjelaskan mengapa ia bekerja seperti ini.

Saat Anda menulis kode untuk memecahkan masalah hari ini, kode itu bisa menjadi masalah besok saat persyaratan berubah. Pemeliharaan selalu harus diperhitungkan dan meningkatkan pemahaman kode sangat penting.

Rudolf Olah
sumber
1
Di sinilah praktik dan prinsip Rekayasa Perangkat Lunak ikut berperan. Persyaratan non-fungsional
hanzolo
0

Keringkasan tidak lagi menjadi kebajikan saat

  • Ada Divisi tanpa cek sebelumnya untuk nol.
  • Tidak ada pemeriksaan untuk nol.
  • Tidak ada pembersihan.
  • COBA CATCH versus melempar rantai makanan di mana kesalahan dapat ditangani.
  • Asumsi dibuat tentang urutan penyelesaian tugas asinkron
  • Tugas menggunakan penundaan alih-alih menjadwal ulang di masa mendatang
  • IO yang tidak perlu digunakan
  • Tidak menggunakan pembaruan optimis
danny117
sumber
Ketika tidak ada Jawaban yang cukup panjang.
1
Ok, ini seharusnya komentar, bukan jawaban. Tapi dia pria baru, jadi setidaknya jelaskan; jangan hanya downvote & lari! Selamat datang, Danny. Saya membatalkan downvote, tapi lain kali beri komentar :-)
Mawg
2
Ok saya sudah memperluas jawabannya dengan memasukkan beberapa hal yang lebih kompleks yang telah saya pelajari dengan susah payah dan cara mudah menulis kode singkat.
danny117
Terima kasih atas sambutannya @Mawg Saya ingin menunjukkan bahwa cek untuk null adalah masalah yang paling sering saya temui dalam kode singkat.
danny117
Saya baru saja mengedit melalui Android dan tidak meminta deskripsi edit. Saya menambahkan pembaruan optimis (deteksi berubah dan memperingatkan)
danny117
0

Jika ini melewati tes unit validasi, maka itu akan baik-baik saja, jika spec baru ditambahkan, tes baru atau implementasi yang ditingkatkan diperlukan, dan itu diperlukan untuk "mengurai" kekasaran kode, saat itulah masalah akan muncul.

Jelas bug dalam kode menunjukkan bahwa ada masalah lain dengan Q / A yang merupakan kelalaian, sehingga fakta bahwa ada bug yang tidak tertangkap adalah memprihatinkan.

Ketika berhadapan dengan persyaratan non-fungsional seperti "keterbacaan" kode, kode tersebut harus didefinisikan oleh manajer pengembangan dan dikelola oleh pengembang utama dan dihormati oleh pengembang untuk memastikan implementasi yang tepat.

Cobalah untuk memastikan implementasi standar kode (standar dan konvensi) untuk memastikan "keterbacaan" dan kemudahan "pemeliharaan". Tetapi jika atribut kualitas ini tidak didefinisikan dan ditegakkan, maka Anda akan berakhir dengan kode seperti contoh di atas.

Jika Anda tidak suka melihat kode semacam ini, maka cobalah untuk mendapatkan tim dalam persetujuan tentang standar dan konvensi implementasi dan tuliskan dan lakukan tinjauan kode acak atau pemeriksaan spot untuk memvalidasi konvensi sedang dihormati.

Hanzolo
sumber