Review Checklist
นี่คือเวอร์ชัน checklist ของสองบทก่อนหน้า: หนึ่งหน้า หกหมวด แต่ละข้อเขียนเป็นคำถามที่คุณถามกับบรรทัดโค้ดที่เจาะจงได้ แต่ละข้อลิงก์ไปบทที่อธิบายว่าทำไมคำถามนั้นถึงสำคัญ เปิดไว้อีกแท็บระหว่างรีวิวจริง
Ownership & Borrowing
หัวข้อที่มีชื่อว่า “Ownership & Borrowing”- ทุก
.clone()มีเหตุผลที่คุณอธิบายได้ในหนึ่งประโยคไหม (“จำเป็นเพราะ X ยังใช้ตัวเดิมอยู่หลังจากนี้”)? — Ownership & Moves - parameter
&Vec<T>หรือ&Stringตรงนี้เป็น&[T]หรือ&strแทนได้ไหม? — Slices Rc<RefCell<T>>ตัวนี้ต้องการหลายเจ้าของและ interior mutability จริงไหม หรือ field ธรรมดากับการยืมแบบ&mutก็พอแล้ว? — Smart Pointers- ทุก
unsafeblock มี comment// SAFETY:อธิบายว่าข้อสมมติไหนทำให้มัน sound ไหม? — The Borrow Checker Model - ทุก lifetime annotation แบบชัดเจนเพิ่มข้อมูลที่ elision จะไม่ infer ให้อยู่แล้วไหม? — Lifetimes
'staticตรงนี้เพราะข้อมูลมีชีวิตอยู่ตลอดโปรแกรมจริง ๆ หรือเพราะมันปิดปาก error? — Lifetimesmem::forgetหรือBox::leakปรากฏอยู่ไหม และถ้ามี leak นั้นตั้งใจและคุ้มกับ comment ที่อธิบายมันไหม? — Drop and RAII
Types & Traits
หัวข้อที่มีชื่อว่า “Types & Traits”- ก่อนจะหยิบ
dyn Traittrait นี้ต้องการ method ที่ returnSelfหรือ generic method ที่ทำให้มัน dyn-incompatible อยู่แล้วไหม? — Trait Objects impl Traitถูกใช้แค่ในตำแหน่ง argument หรือ return เท่านั้น ไม่เคยเป็น field type ของ struct ใช่ไหม? — Generics- ทุก
#[derive(Copy)]อยู่บน type ที่เล็กพอที่ implicit copy จะถูกจริง ๆ ไหม? — Structs - float (
f32/f64) ถูกใช้เป็นHashMap/HashSetkey หรือถูกเปรียบเทียบด้วย==ตรง ๆ บ้างไหม? — HashMap
Errors & Panics
หัวข้อที่มีชื่อว่า “Errors & Panics”- ทุก
.unwrap()/.expect(...)อยู่บนค่าที่ infallible จริง ๆ ตรงนี้ ไม่ใช่ input ผู้ใช้, I/O, หรือ network response ใช่ไหม? — Panics - มี
matchบน enum ตัวไหนมี_ =>arm ไหม และถ้ามี enum นั้นมีโอกาสโตขึ้นเป็น variant ใหม่ในอนาคตได้จริงไหม? — Pattern Matching .is_some()/.is_ok()ตามด้วย.unwrap()แยกกันคนละขั้นบ้างไหม ในที่ที่if let/matchทำทั้งสองอย่างในขั้นตอนเดียวได้? — OptionBox<dyn Error>ถูกใช้เป็น public return type ของ library ในที่ที่ผู้เรียกอาจต้อง match กับ failure เฉพาะเจาะจงไหม? — Custom Errors- มี
panic!ตัวไหนครอบคลุมเงื่อนไขที่ผู้เรียกอาจอยากกู้คืนได้อย่างสมเหตุสมผลไหม? — Result
Iterators & Collections
หัวข้อที่มีชื่อว่า “Iterators & Collections”for i in 0..collection.len()loop ตัวไหนใช้iแค่ index collection เดียวกันนั้นไหม? — Iterators.collect()ถูกใช้ต่อทันทีด้วย method รูปร่าง iterator อีกตัว (.contains(...),.any(...),.len()) ไหม? — Iterators- string byte-slice ตัวไหน (
&s[a..b]) สมมติว่าเป็น ASCII โดยไม่เช็คว่าa/bตกบนcharboundary สำหรับ input ที่ไม่ใช่ ASCII ไหม? — Vec and String ascast ระหว่าง integer type ตัวไหนเสี่ยง truncate อย่างเงียบ ๆ บน input ที่ไม่ใช่ literal เล็ก ๆ ไหม? — Data Types
Concurrency & Async
หัวข้อที่มีชื่อว่า “Concurrency & Async”Arc<Mutex<T>>ตัวนี้ต้องมีชีวิตอยู่นานกว่าฟังก์ชันที่ spawn thread ของมันไหม หรือthread::scopeจะตัดความต้องการArcออกได้? — Threads- integer ตัวเดียวที่
Mutexปกป้องอยู่เป็น atomic ธรรมดาแทนได้ไหม? — Threads std::sync::MutexGuardตัวไหนยังอยู่ใน scope ข้าม.awaitpoint ไหม? — Async Introasync fnตัวไหนเรียกthread::sleepหรือ blocking call อื่นแทนที่จะเป็น async-aware equivalent ไหม? — Async Intrompsc::channel()(ไม่มีขอบเขต) ถูกใช้ในที่ที่ producer อาจเร็วกว่า consumer ได้จริงไหม? — Channels
Cargo & Tests
หัวข้อที่มีชื่อว่า “Cargo & Tests”- ทุก
#[should_panic]ระบุexpected = "..."ไหม? — Cargo Tests - ทุก body ของฟังก์ชัน
#[test]มีassert!/assert_eq!/assert_ne!อย่างน้อยหนึ่งตัวไหม? — Cargo Tests cargo clippy -- -D warningsรันสะอาดไหม และมันถูกรันด้วย AI-smell lint จาก tool ladder เปิดใช้งานจริงหรือยัง? — Verification Tools- resolver กับ
rust-versionในCargo.tomlถูกตั้งไว้อย่างตั้งใจ ไม่ปล่อยให้เป็น default ใช่ไหม? — Cargo Tests
Compiler พูดว่าอย่างไร: การแก้ไข Vec ระหว่างที่กำลัง Iterate มันอยู่
หัวข้อที่มีชื่อว่า “Compiler พูดว่าอย่างไร: การแก้ไข Vec ระหว่างที่กำลัง Iterate มันอยู่”checklist ข้อหนึ่ง — “มีอะไรดูเหมือนจะทำให้ iterator invalid กลาง loop ไหม” — คุ้มที่จะเห็นมันถูกปฏิเสธตรง ๆ เพราะ Rust ปฏิเสธสิ่งนี้ที่ compile time แทนที่จะปล่อยให้มันทำลาย memory ที่ runtime แบบที่จะเกิดในภาษาที่มีกฎ aliasing หลวมกว่านี้:
// @expect-error E0502fn main() { let mut v = vec![1, 2, 3]; for x in &v { v.push(*x); }}error[E0502]: cannot borrow `v` as mutable because it is also borrowed as immutable --> src/main.rs:4:9 |3 | for x in &v { | -- | | | immutable borrow occurs here | immutable borrow later used here4 | v.push(*x); | ^^^^^^^^^^ mutable borrow occurs herefor x in &v ถือ immutable borrow ของ v ตลอดทั้ง loop; v.push(*x) ต้องการ mutable borrow ซึ่งอยู่ร่วมกับมันไม่ได้ ข้อผิดพลาดเฉพาะนี้ — การ append เข้า collection ระหว่างที่ iterate มันอยู่ — เป็น undefined behavior ในภาษาอื่นหลายภาษา (backing array ที่ resize อาจทำให้ x ชี้ไปที่ memory ที่ถูกปล่อยแล้วหรือเก่าไป) แต่ที่นี่เป็น compile error แทน
เวอร์ชันสั้นแบบ Copy-Paste ได้
หัวข้อที่มีชื่อว่า “เวอร์ชันสั้นแบบ Copy-Paste ได้”สำหรับการผ่านแบบเร็วหรือ PR description นี่อัดทุกคำถามข้างบนให้เหลือแค่วลี:
Ownership: unexplained .clone() / &Vec or &String params / Rc<RefCell> as default / unsafe without SAFETY / decorative lifetimes / 'static-to-silence / mem::forget or Box::leakTypes: dyn where impl/enum would do / impl Trait in a field / Copy on a large struct / float as a HashMap key or compared with ==Errors: unwrap on fallible input / _ => on a growable enum / is_some+unwrap / Box<dyn Error> in a library API / panic! on a recoverable conditionIterators: index loop instead of iterator / needless collect / byte-slice on non-ASCII / as-cast truncation riskConcurrency: Arc<Mutex> where scope/atomic would do / lock held across .await / thread::sleep in async / unbounded channelCargo & tests: should_panic without expected / test with no assert / clippy not run with -D warnings / resolver and rust-version left to defaultGrep List
หัวข้อที่มีชื่อว่า “Grep List”ก่อนจะอ่าน body ของฟังก์ชันแม้แต่ตัวเดียว รันคำสั่งนี้บนไฟล์ (หรือ diff) เพื่อดูว่ารายการที่ signal สูงสุดของ checklist กระจุกตัวอยู่ตรงไหน:
grep -nE 'clone\(\)|unwrap\(\)|expect\(|unsafe|'"'"'static|Rc<RefCell|Box<dyn Error|as u|_ =>|mem::forget|#\[allow|\.lock\(\)\.unwrap\(\)' src/**/*.rsไม่กี่ hit กระจายอยู่ในไฟล์ใหญ่ ๆ เป็นเรื่องปกติ hit ห้าหกตัวกระจุกอยู่ในฟังก์ชันเดียวคือวิธีที่เร็วที่สุดของ checklist ในการชี้ว่าควรเริ่มอ่านอย่างละเอียดตรงไหน
struct Order { items: Vec<(String, u32)>, // (name, price in cents)}
// &[...] not &Vec<...>, &str not &String -- accepts strictly more callersfn total_cents(items: &[(String, u32)]) -> u32 { items.iter().map(|(_, price)| price).sum()}
fn format_receipt(order: &Order) -> String { let mut lines = Vec::new(); for (name, price) in &order.items { // iterator, not an index loop lines.push(format!("{name}: {price}")); } lines.push(format!("total: {}", total_cents(&order.items))); lines.join("\n")}
fn main() { let order = Order { items: vec![ ("coffee".to_string(), 350), ("bagel".to_string(), 275), ], }; println!("{}", format_receipt(&order));}Compiling…
โค้ดนี้ผ่านทุกข้อใน checklist ข้างบน: ไม่มี .clone() เลยสักที่, &[(String, u32)] แทน &Vec<...>, iterator ธรรมดาแทน index loop, ไม่มี .unwrap() บนอะไรที่ fallible เลย (ไม่มีอะไร fallible ตรงนี้เลย) และไม่มี suppression เลย การจำได้ว่าการผ่านแบบสะอาดหน้าตาเป็นอย่างไรมีประโยชน์พอ ๆ กับการจำได้ว่ากลิ่นไม่ดีหน้าตาเป็นอย่างไร — มันคือ baseline ที่ catalog ส่วนที่เหลือของโมดูลนี้ถูกวัดเทียบด้วย
Mental Model
หัวข้อที่มีชื่อว่า “Mental Model”Checklist แลก depth กับ coverage: มันจะไม่จับ logic bug ที่ subtle เฉพาะ domain ของ codebase นี้ แต่มันเผยรูปร่างของปัญหาสองโหลเดิมได้อย่างสม่ำเสมอในไฟล์ Rust ใด ๆ ในเวลาคงที่ ไม่ว่าคุณจะคุ้นเคยกับ codebase นี้แค่ไหน นั่นแหละคือการแลกที่การอ่านผ่านครั้งแรกบนโค้ดที่ไม่คุ้นเคย (หรือโค้ดที่ AI สร้าง) ควรทำ — หาก่อนว่าความเสี่ยงกระจุกอยู่ตรงไหนก่อนที่จะใช้เวลาอ่านลึกกับฟังก์ชันใดฟังก์ชันหนึ่ง
Trade-offs
หัวข้อที่มีชื่อว่า “Trade-offs”การรัน grep list กับ checklist ทุกครั้งที่รีวิวเพิ่มเวลาสองสามนาทีให้กับการผ่านที่ “แค่อ่าน diff” จะไม่ใช้เวลาแบบนั้น — และบน diff ที่กลายเป็นว่าสะอาดล้วน ๆ เวลานั้นไม่ให้ finding อะไรเลย ทางเลือกคือข้ามไปอ่าน logic ตรง ๆ เร็วกว่าบน diff ที่สะอาด และแย่กว่าพอดีบน diff ที่มันสำคัญที่สุด: ไฟล์ที่มี pattern พวกนี้หลายตัวกระจุกอยู่ด้วยกัน ซึ่งการอ่านแบบไม่มีโครงสร้างมีโอกาสน้อยที่สุดที่จะสังเกตว่ามันเป็นกลุ่มก้อน แทนที่จะเห็นเป็นทางเลือกแยกกันที่ให้อภัยได้ทีละอัน
Common Misconceptions
หัวข้อที่มีชื่อว่า “Common Misconceptions”- “checklist ข้อไหนถูก flag แปลว่าโค้ดผิด” มันแปลว่าโค้ดนั้นสมควรได้รับคำถามที่เจาะจง หลายข้อ (
.clone()ที่จำเป็นจริง ๆ,Box<dyn Error>ที่เปิดกว้างอย่างชอบธรรม) คาดว่าจะผ่านคำถามนั้นโดยไม่ถูก flag - “ถ้า clippy สะอาด checklist ก็ไม่จำเป็น” หลายข้อใน checklist — channel ไม่มีขอบเขต,
Arc<Mutex<T>>ในที่ที่ scope จะพอ, test ที่ไม่มี assertion — ไม่มี clippy lint ที่ตรงกันเลย checklist กับ tool ladder (บทถัดไป) ครอบคลุมพื้นที่ที่ต่างกัน ทับกันแค่บางส่วน - “grep list จับทุกอย่างที่ checklist ครอบคลุม” มันจับเฉพาะส่วนที่ grep ได้ทาง syntax เท่านั้น
_ =>catch-all, lifetime ประดับ, และ “design นี้ต้องการหลายเจ้าของจริงไหม” ล้วนต้องการการอ่าน ไม่ใช่แค่การจับคู่ text
อ่านโค้ดที่ AI เขียน
หัวข้อที่มีชื่อว่า “อ่านโค้ดที่ AI เขียน”การรัน grep list บนไฟล์ที่ AI สร้างมักจะเผย hit หลายตัวในฟังก์ชันเดียว ซึ่งเป็น signal “กลุ่มก้อน” ที่ checklist ถูกออกแบบมาให้จับพอดี
🐛 หา bug ในโค้ดนี้
use std::rc::Rc;use std::cell::RefCell;
struct Cache { data: Rc<RefCell<Vec<String>>>,}
fn add_entry(cache: Rc<RefCell<Vec<String>>>, entry: String) { cache.borrow_mut().push(entry);}
fn print_all(cache: Rc<RefCell<Vec<String>>>) { for item in cache.borrow().iter() { println!("{item}"); }}
fn main() { let cache = Rc::new(RefCell::new(Vec::new())); add_entry(cache.clone(), "first".to_string()); add_entry(cache.clone(), "second".to_string()); print_all(cache.clone());}เฉลย
การ grep ไฟล์นี้หา term ของ checklist เจอ Rc<RefCell สามครั้งและ .clone() สามครั้ง ทั้งหมดอยู่ในไฟล์ 20 บรรทัดที่มี thread เดียวและไม่มีความต้องการ shared-ownership จริง ๆ เลย — main เป็นผู้เรียกตัวเดียว และมันเรียกทุกอย่างตามลำดับ ไม่มีอะไรตรงนี้ต้องการเจ้าของสองคนมีชีวิตอยู่พร้อมกันหรือ interior mutability ข้าม alias เลย &mut Vec<String> ธรรมดาที่ส่งแบบ mutable reference ทำงานเดียวกันได้โดยไม่มี Rc, ไม่มี RefCell, และไม่มี .clone() เลยสักที่: fn add_entry(cache: &mut Vec<String>, entry: String) เรียกเป็น add_entry(&mut cache, "first".to_string()) grep list ไม่ได้บอกสิ่งนี้ด้วยตัวมันเอง — Rc<RefCell สาม hit ในไฟล์เล็ก ๆ เดียวคือ signal ที่บอกว่า “ไปอ่านฟังก์ชันนี้แล้วถามว่ามันจำเป็นต้องอยู่ตรงนี้ไหม” และการอ่านมันคือสิ่งที่ยืนยันว่าคำตอบคือไม่