Logic Analysis

วิเคราะห์ Logic Programming

ทำไมถึงเขียนแบบนี้? แล้วจะปรับปรุงให้ดีขึ้นได้อย่างไร?

Pattern

1. Nested SELECT แทน JOIN

โค้ดปัจจุบัน (Nested SELECT)
select  tccom100.*
from    tccom100
where   tccom100.bpid = {:sccoa003.stbp}
selectdo
endselect

select  tccom130.*
from    tccom130
where   tccom130.cadr = {:tccom100.cadr}
selectdo
endselect

if tccom130.ccty = "THA" then
    coa.type = 1
endif
วิธีที่ดีกว่า (Single JOIN)
select  tccom100.*, tccom130.*
from    tccom100
JOIN    tccom130 
  ON    tccom130.cadr = tccom100.cadr
where   tccom100.bpid = {:sccoa003.stbp}
selectdo
    if tccom130.ccty = "THA" then
        coa.type = 1
    else
        coa.type = 2
    endif
endselect
ทำไมถึงเขียนแบบนี้?

ใน 4GL รุ่นเก่า การเขียน SELECT ซ้อน SELECT เป็นวิธีปกติเพราะ JOIN syntax ยังไม่รองรับ แต่ใน LN 10.6+ รองรับ JOIN แล้ว

ข้อดีของ JOIN: ลดจำนวน Database Round-trip, อ่านง่ายกว่า, Performance ดีกว่า
ข้อควรระวัง: ถ้าข้อมูล Address ไม่มี อาจ Error ได้ — ควรใช้ LEFT JOIN
Pattern

2. ใช้ Flag Variables ควบคุม Flow

โค้ดปัจจุบัน
boolean  first.time
boolean  first.time.2

| ใน loop:
if first.time = true then
    rpt = brp.open("rsccoa060301000","S",0)
    first.time = false
endif

| อีกที่:
if first.time.2 = true then
    rpt2 = brp.open("rsccoa060401000","S",0)
    first.time.2 = false
endif
วิธีที่ดีกว่า
| ใช้ค่า rpt เอง เป็น flag
| (rpt = 0 หมายถึง ยังไม่เปิด)
long  rpt   | init = 0
long  rpt2  | init = 0

| ใน loop:
if coa.type = 1 and rpt = 0 then
    rpt = brp.open("rsccoa060301000","S",0)
endif

if coa.type = 2 and rpt2 = 0 then
    rpt2 = brp.open("rsccoa060401000","S",0)
endif
ทำไมถึงเขียนแบบนี้?

ผู้พัฒนาใช้ตัวแปร first.time และ first.time.2 เป็น flag เพื่อป้องกันไม่ให้เปิดรายงานซ้ำ เพราะ brp.open() ควรถูกเรียกแค่ครั้งเดียว

ปรับปรุง: ใช้ค่า rpt เอง (0 = ยังไม่เปิด) แทนการสร้าง boolean แยก — ลดตัวแปรที่ต้องจัดการ
ตั้งชื่อให้ดีกว่า: first.time.2 ไม่สื่อความหมาย — ควรเป็น export.rpt.opened
Anti-Pattern

3. IF ซ้อนลึก (Deep Nesting) ในส่วน Specification

โค้ดปัจจุบัน (ซ้อน 7+ ชั้น)
if sccoa004.prto = scspec.prt.opt.min then
  if qmptc001.ctyp = qmptc.ctyp.fraction then
    if trim$(sccoa004.char) = "SP_GRAV2"
       or ... then
        rpt.spec = edit$(..., "ZZZ9VD9999")
    else
        rpt.spec = "Min." & edit$(...)
    endif
  else
    if qmptc001.ctyp = qmptc.ctyp.integer then
        rpt.spec = "Min." & edit$(...)
    else
        rpt.spec = "Min." & edit$(...)
    endif
  endif
else
  if sccoa004.prto = scspec.prt.opt.max then
    ... | ซ้อนอีก 5 ชั้น
  endif
endif
วิธีที่ดีกว่า (แยกฟังก์ชัน)
| แยกเป็นฟังก์ชันย่อย
function string format.spec.value(
    domain tcdate val, 
    domain tcchar char.code,
    domain tclong ctyp)
{
    if is.high.precision.char(char.code) then
        return(edit$(val, "ZZZ9VD9999"))
    else
        if ctyp = qmptc.ctyp.fraction then
            return(edit$(val, "ZZZZZ9VD99"))
        else
            return(edit$(val, "ZZZZZZZZZ9"))
        endif
    endif
}

| เรียกใช้:
if prto = min then
    rpt.spec = "Min. " & format.spec.value(...)
else if prto = max then
    rpt.spec = "Max. " & format.spec.value(...)
else if prto = range then
    rpt.spec = format.spec.value(llmt) 
             & " - " & format.spec.value(ulmt)
endif
ทำไมถึงเขียนแบบนี้?

เหตุผลหลักคือโปรแกรมถูกแก้ไขสะสมหลายปี (2009–2026) แต่ละครั้งเพิ่ม if เข้าไปเรื่อยๆ โดยไม่ได้ Refactor ใหม่ทั้งหมด ทำให้โค้ดซ้อนลึกมาก

แยก function: สร้างฟังก์ชัน format.spec.value() จัดการ format ให้ แล้วเรียกใช้ซ้ำ
Early return: ตรวจเงื่อนไขแล้ว return ทันที แทนที่จะซ้อน else
Code Smell

4. Hardcoded Values (ค่าคงที่ในโค้ด)

โค้ดปัจจุบัน
| Hardcoded BP check
if sccoa003.stbp(1;6) = "110472" then
    Vender_No = "VenderSiteNo: 100786789"
    Vender_Manu = "VenderSiteCity:SamutPrakan TH"
endif

| Hardcoded item check
if trim$(tcibd001.item) = "03020" then
    rpt.packaging = "IBC"
endif

| Hardcoded label
rpt.lbl.22 = "   Suttichai Chaisupitanarak"
วิธีที่ดีกว่า
| ย้ายไปเก็บในตาราง Configuration
| สร้างตาราง sccoa010 (BP Config)
select sccoa010.vender_no, sccoa010.vender_manu
from   sccoa010
where  sccoa010.bpid = {:sccoa003.stbp}
selectdo
    Vender_No = sccoa010.vender_no
    Vender_Manu = sccoa010.vender_manu
endselect

| สร้างตาราง sccoa011 (Item Packaging)
select sccoa011.packaging
from   sccoa011
where  sccoa011.item = {:tcibd001.item}
selectdo
    rpt.packaging = sccoa011.packaging
endselect

| ชื่อ QA Manager ดึงจาก DB แล้ว (ดีอยู่แล้ว)
rpt.lbl.22 = tccom001.namb
ทำไมถึงเขียนแบบนี้?

เป็น Quick Fix สำหรับลูกค้าเฉพาะราย (Nestlé = BP 110472) ผู้พัฒนาฝังค่าตรงๆ เพราะเร็วที่สุด แต่เมื่อมีลูกค้าใหม่จะต้องแก้โค้ดทุกครั้ง

ใช้ Configuration Table: สร้างตารางเก็บค่า Config แทน — จะเพิ่ม/แก้ไขได้โดยไม่ต้องแก้โค้ด
ข้อจำกัด: ใน ERP การขอสร้างตารางใหม่มักต้องผ่านกระบวนการ Change Management ที่ซับซ้อน จึงเลือก Hardcode แทน
Missing

5. ขาด Error Handling

โค้ดปัจจุบัน
| ไม่มี selectempty ในหลาย SELECT
select  tcibd001.*
from    tcibd001
where   tcibd001.item = {:sccoa003.item}
selectdo
endselect
| ถ้าไม่พบ item → tcibd001.dsca จะเป็นค่าว่าง
| → รายงานออกมาไม่มีชื่อสินค้า

| ไม่มีการดักจับ Error
| ถ้า brp.open() fail → โปรแกรมจะทำงานต่อ
| → ค่า rpt อาจเป็น 0 หรือ -1
วิธีที่ดีกว่า
select  tcibd001.*
from    tcibd001
where   tcibd001.item = {:sccoa003.item}
selectdo
selectempty
    | Handle case: Item ไม่พบ
    item.dsca = "[Item not found: " 
              & trim$(sccoa003.item) & "]"
endselect

| ตรวจสอบผลการเปิดรายงาน
rpt = brp.open("rsccoa060301000", "S", 0)
if rpt <= 0 then
    message("Cannot open report template")
    return  | หยุดทำงาน
endif
ทำไมถึงเขียนแบบนี้?

ใน ERP ข้อมูลมักถูกตรวจสอบแล้วก่อนถึงขั้นตอนพิมพ์ COA (Master Data ต้องสมบูรณ์) จึงสมมติว่าข้อมูลจะมีเสมอ แต่ในทางปฏิบัติ ข้อมูลอาจหายได้

เพิ่ม selectempty: จัดการกรณีไม่พบข้อมูลทุก SELECT
ตรวจค่า return: เช็คผลลัพธ์จาก brp.open() ก่อนส่งข้อมูล
Architecture

6. ฟังก์ชันยาวเกินไป (God Function)

ปัญหาหลัก

ฟังก์ชัน read.main.table() ยาว 466 บรรทัด (บรรทัด 178–644) ทำทุกอย่างในที่เดียว — ควรแยกออกเป็นฟังก์ชันย่อย

read.main.table()
466 บรรทัด — ทำทุกอย่าง
Refactor เป็น
read.main.table()
ควบคุม flow หลัก ~50 บรรทัด
get.bp.and.address()
ดึงข้อมูล BP + Address
open.report()
เปิดรายงานตาม Local/Export
process.detail()
ประมวลผล Detail แต่ละ record
format.result()
จัดรูปแบบผลทดสอบ
calc.specification()
คำนวณ Specification
fill.remaining.rows()
เติมบรรทัดว่างครบหน้า
Summary

สรุปการประเมินคุณภาพโค้ด

4/10
โครงสร้าง
5/10
อ่านง่าย
3/10
Error Handling
7/10
ฟังก์ชันการทำงาน
6/10
ตั้งชื่อตัวแปร
8/10
Change Log

สรุป: โปรแกรมทำงานได้ถูกต้องและใช้งานจริงมานาน แต่โครงสร้างซับซ้อนจากการแก้ไขสะสมหลายปี จุดแข็งคือ Change Log ดี มีหมายเหตุทุกการแก้ไข จุดอ่อนคือ Error Handling น้อย และฟังก์ชันหลักยาวเกินไป

ดูวิธีแปลงเป็น SQL Query