- บทนำ
- ตัวแปร
- ใช้ชื่อตัวแปรที่มีความหมายและอ่านออกเสียงได้
- ใช้ศัพท์เดียวกันสำหรับตัวแปรชนิดเดียวกัน
- ใช้ชื่อที่ค้นหาเจอได้ (ภาค 1)
- ใช้ชื่อที่ค้นหาเจอได้ (ภาค 2)
- ใช้ตัวแปรที่อธิบายตัวเองได้
- หลีกเลี่ยงโค้ดที่ซ้อนลงไปลึกเกินไปและคืนค่าตั้งแต่ต้น (ภาค 1)
- หลีกเลี่ยงโค้ดที่ซ้อนลงไปลึกเกินไปและคืนค่าตั้งแต่ต้น (ภาค 2)
- หลีกเลี่ยงการต้องไล่ลำดับเอง
- อย่าเพิ่มสิ่งที่ไม่จำเป็น
- ให้อาร์กิวเมนต์มีค่าเริ่มต้น แทนที่การใช้ทางลัดหรือใช้การเช็คเงื่อนไข
- Comparison
- ฟังก์ชัน
- อาร์กิวเมนท์ในฟังก์ชัน (2 ตัวหรือน้อยกว่าจะดีมาก)
- ฟังก์ชันควรจะทำงานเพียงอย่างเดียว
- ชื่อฟังก์ชันควรจะสื่อว่ามันทำอะไร
- ฟังก์ชันควรจะมีสาระสำคัญแค่ระดับเดียว
- อย่าใช้แฟล็ก (Flag) เป็นพารามิเตอร์ของฟังก์ชัน
- ระวังผลข้างเคียง (size effects)
- อย่าเขียนฟังก์ชันแบบโกลบอล
- อย่าใช้แพทเทิน ซิงเกิลตัน (Singleton)
- ซ่อนการทำงานของเงื่อนไข
- อย่าใช้เงื่อนไขที่เป็นเชิงลบ
- เลิกใช้เงื่อนไขไปเลย
- หลีกเลี่ยงการตรวจสอบชนิดข้อมูล (ภาค 1)
- หลีกเลี่ยงการตรวจสอบชนิดข้อมูล (ภาค 2)
- เอาโค้ดที่ตายแล้วออกไป
- อ็อบเจคและโครงสร้างข้อมูล
- คลาส
- SOLID
- Don’t repeat yourself (DRY)
- การแปล
หลักการทางวิศวกรรมซอฟแวร์ จากหนังสือของโรเบิร์ต ซี. มาร์ติน เรื่อง Clean Code, โดยปรับตัวอย่างให้เป็นภาษา PHP เอกสารนี้ไม่ใช่แนวทางการเขียนโปรแกรมให้สวยงาม แต่มันเป็นแนวทางในการสร้างซอฟแวร์ด้วย PHP ที่อ่านง่าย นำกลับมาใช้ได้ และสามารถปรับปรุงได้
คุณไม่จำเป็นต้องยึดถือหลักการในเอกสารนี้อย่างเข้มงวดและไม่จำเป็นต้องเห็นด้วยในบางเรื่อง เนื้อหานี้เป็นเพียงแนวทางเท่านั้น หากแต่มันเป็นผลผลิตจากการเก็บเกี่ยวประสบการณ์หลายปีของผู้เขียนหนังสือ Clean Code
ได้รับแรงบันดาลใจจาก clean-code-javascript
ถึงแม้ว่านักพัฒนาส่วนใหญ่ยังใช้ PHP 5 แต่ตัวอย่างในบทความนี้ใช้ได้กับ PHP 7.1 หรือใหม่กว่าเท่านั้น
ไม่ดี:
$ymdstr = $moment->format('y-m-d');ดี:
$currentDate = $moment->format('y-m-d');ไม่ดี:
getUserInfo();
getUserData();
getUserRecord();
getUserProfile();ดี:
getUser();เราต้องอ่านโค้ดมากกว่าเขียน จึงจำเป็นที่โค้ดของเราต้องอ่านง่ายและหาสิ่งที่ต้องการได้ง่าย ตั้งชื่อตัวแปรที่เข้าใจยากมันทำร้ายคนอ่าน ใช้ชื่อที่ค้นหาได้
ไม่ดี:
// 448 เอาไว้ทำอะไร?$result = $serializer->serialize($data, 448);ดี:
$json = $serializer->serialize($data, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE);ไม่ดี:
// 4 เอาไว้ทำอะไรเนี่ย?if ($user->access & 4) {
// ...
}ดี:
class User
{
constACCESS_READ = 1;
constACCESS_CREATE = 2;
constACCESS_UPDATE = 4;
constACCESS_DELETE = 8;
}
if ($user->access & User::ACCESS_UPDATE) {
// do edit ...
}ไม่ดี:
$address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,]+,\s*(.+?)\s*(\d{5})$/';
preg_match($cityZipCodeRegex, $address, $matches);
saveCityZipCode($matches[1], $matches[2]);ไม่เลว::
ดีขึ้น แต่เรายังต้องพึ่งพา regex มากไป
$address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,]+,\s*(.+?)\s*(\d{5})$/';
preg_match($cityZipCodeRegex, $address, $matches);
[, $city, $zipCode] = $matches;
saveCityZipCode($city, $zipCode);ดี::
ลดการพึ่งพา regex ด้วยการตั้งชื่อให้แพทเทินย่อย
$address = 'One Infinite Loop, Cupertino 95014';
$cityZipCodeRegex = '/^[^,]+,\s*(?<city>.+?)\s*(?<zipCode>\d{5})$/';
preg_match($cityZipCodeRegex, $address, $matches);
saveCityZipCode($matches['city'], $matches['zipCode']);คำสั่ง if else มากเกินไปทำให้ไล่โค้ดได้ยาก ความเฉพาะเจาะจงดีกว่าต้องคาดเดา
ไม่ดี:
functionisShopOpen($day): bool
{
if ($day) {
if (is_string($day)) {
$day = strtolower($day);
if ($day === 'friday') {
returntrue;
} elseif ($day === 'saturday') {
returntrue;
} elseif ($day === 'sunday') {
returntrue;
} else {
returnfalse;
}
} else {
returnfalse;
}
} else {
returnfalse;
}
}ดี:
functionisShopOpen(string$day): bool
{
if (empty($day)) {
returnfalse;
}
$openingDays = [
'friday', 'saturday', 'sunday'
];
returnin_array(strtolower($day), $openingDays, true);
}ไม่ดี:
functionfibonacci(int$n)
{
if ($n < 50) {
if ($n !== 0) {
if ($n !== 1) {
returnfibonacci($n - 1) + fibonacci($n - 2);
} else {
return1;
}
} else {
return0;
}
} else {
return'Not supported';
}
}ดี:
functionfibonacci(int$n): int
{
if ($n === 0 || $n === 1) {
return$n;
}
if ($n > 50) {
thrownew \Exception('Not supported');
}
returnfibonacci($n - 1) + fibonacci($n - 2);
}อย่าบังคับให้คนที่อ่านโค้ดคุณต้องไล่เรียงว่าตัวแปรนั้นหมายถึงอะไร ประกาศแบบเฉพาะเจาะจงดีกว่า
ไม่ดี:
$l = ['Austin', 'New York', 'San Francisco'];
for ($i = 0; $i < count($l); $i++) {
$li = $l[$i];
doStuff();
doSomeOtherStuff();
// ...// ...// ...// Wait, what is `$li` for again?dispatch($li);
}ดี:
$locations = ['Austin', 'New York', 'San Francisco'];
foreach ($locationsas$location) {
doStuff();
doSomeOtherStuff();
// ...// ...// ...dispatch($location);
}ถ้าชื่อคลาสหรือออบเจคบอกอะไรบางอย่างอยู่แล้ว อย่าใส่มันในชื่อตัวแปรภายในอีก
ไม่ดี:
class Car
{
public$carMake;
public$carModel;
public$carColor;
//...
}ดี:
class Car
{
public$make;
public$model;
public$color;
//...
}ไม่ดี:
ไม่ดีเพราะ $breweryName สามารถเป็น NULL.
functioncreateMicrobrewery($breweryName = 'Hipster Brew Co.'): void
{
// ...
}ไม่เลว:
โค้ดชุดนี้เข้าใจได้ง่ายกว่าชุดที่แล้ว มีการตรวจสอบตัวแปร
functioncreateMicrobrewery($name = null): void
{
$breweryName = $name ?: 'Hipster Brew Co.';
// ...
}ดี:
ถ้าคุณใช้ PHP 7+ คุณสามารถใช้ type hinting และแน่ใจได้ว่า $breweryName จะไม่เป็น NULL.
functioncreateMicrobrewery(string$breweryName = 'Hipster Brew Co.'): void
{
// ...
}Not good:
The simple comparison will convert the string in an integer.
$a = '42';
$b = 42;
if ($a != $b) {
// The expression will always pass
}The comparison $a != $b returns FALSE but in fact it's TRUE!
The string 42 is different than the integer 42.
Good:
The identical comparison will compare type and value.
$a = '42';
$b = 42;
if ($a !== $b) {
// The expression is verified
}The comparison $a !== $b returns TRUE.
การจำกัดจำนวนพารามิเตอ์ในฟังก์ชันเป็นสิ่งที่สำคัญมาก เพราะจะทำให้ทดสอบฟังก์ชันนั้นได้ง่ายขึ้น หากมีมากกว่า 3 มักจะเพิ่มความยุ่งยากเพราะต้องทดสอบหลายเคสกับอาร์กิวเมนท์แต่ละตัว
ไม่มีอาร์กิวเมนท์เลยถือเป็นเคสอุดมคติ มีหนึ่งหรือสองตัวยังพอใช้ได้ และควรหลีกเลี่ยงการมีสามตัวหรือมากกว่า โดยทั่วไปแล้วถ้ามีมากกว่า 2 ตัวแสดงว่าฟังก์ชันของคุณทำงานมากเกินไป ถ้าไม่ใช่นั้นส่วนใหญ่แล้วการใช้อ๊อบเจคมาเป็นอาร์กิวเมนท์เป็นสิ่งที่ดีกว่า
ไม่ดี:
functioncreateMenu(string$title, string$body, string$buttonText, bool$cancellable): void
{
// ...
}ดี:
class MenuConfig
{
public$title;
public$body;
public$buttonText;
public$cancellable = false;
}
$config = newMenuConfig();
$config->title = 'Foo';
$config->body = 'Bar';
$config->buttonText = 'Baz';
$config->cancellable = true;
functioncreateMenu(MenuConfig$config): void
{
// ...
}นี่เป็นกฏที่สำคัญมากกฏหนึ่งในวิศวกรรมซอฟแวร์เลยทีเดียว ถ้าฟังก์ชันทำงานมากกว่าหนึ่งอย่าง มันจะเขียนยาก ทดสอบยาก และเข้าใจความหมายได้ยาก เมื่อคุณแยกให้มันทำงานอย่างเดียว มันจะปรับปรุงได้ง่ายและโค้ดของคุณจะสะอาดขึ้น ถ้าคุณยึดถือคำแนะนำนี้เป็นสำคัญ คุณจะนำหน้าเดเวลลอปเปอร์หลายคนทีเดียว
ไม่ดี:
functionemailClients(array$clients): void
{
foreach ($clientsas$client) {
$clientRecord = $db->find($client);
if ($clientRecord->isActive()) {
email($client);
}
}
}ดี:
functionemailClients(array$clients): void
{
$activeClients = activeClients($clients);
array_walk($activeClients, 'email');
}
functionactiveClients(array$clients): array
{
returnarray_filter($clients, 'isClientActive');
}
functionisClientActive(int$client): bool
{
$clientRecord = $db->find($client);
return$clientRecord->isActive();
}ไม่ดี:
class Email
{
//...publicfunctionhandle(): void
{
mail($this->to, $this->subject, $this->body);
}
}
$message = newEmail(...);
// อะไรเนี่ย? จัดการข้อความ? นี่เรากำลังจะเขียนลงไฟล์เหรอ?$message->handle();ดี:
class Email {
//...publicfunctionsend(): void
{
mail($this->to, $this->subject, $this->body);
}
}
$message = newEmail(...);
// เคลียร์และชัดเจน$message->send();ถ้าคุณมีสาระสำคัญมากกว่าหนึ่งระดับ ฟังก์ชันของคุณมักจะทำงานมากเกินไป การแยกฟังก์ชันออกทำให้นำกลับมาใช้ใหม่ได้และทำให้ทดสอบง่ายขึ้น
ไม่ดี:
functionparseBetterJSAlternative(string$code): void
{
$regexes = [
// ...
];
$statements = explode('', $code);
$tokens = [];
foreach ($regexesas$regex) {
foreach ($statementsas$statement) {
// ...
}
}
$ast = [];
foreach ($tokensas$token) {
// lex...
}
foreach ($astas$node) {
// parse...
}
}นี่ก็ยังไม่ดี:
เราแยกฟังก์ชันออกมา แต่ฟังก์ชัน parseBetterJSAlternative() ก็ยังซับซ้อนและยังทดสอบไม่ได้อยู่ดี
functiontokenize(string$code): array
{
$regexes = [
// ...
];
$statements = explode('', $code);
$tokens = [];
foreach ($regexesas$regex) {
foreach ($statementsas$statement) {
$tokens[] = /* ... */;
}
}
return$tokens;
}
functionlexer(array$tokens): array
{
$ast = [];
foreach ($tokensas$token) {
$ast[] = /* ... */;
}
return$ast;
}
functionparseBetterJSAlternative(string$code): void
{
$tokens = tokenize($code);
$ast = lexer($tokens);
foreach ($astas$node) {
// parse...
}
}ดี:
ทางออกที่ดีที่สุดคือย้ายดีเพนเดนซี่ (dependencies) ของ parseBetterJSAlternative() ออกมาเพื่อให้ทดสอบได้
class Tokenizer
{
publicfunctiontokenize(string$code): array
{
$regexes = [
// ...
];
$statements = explode('', $code);
$tokens = [];
foreach ($regexesas$regex) {
foreach ($statementsas$statement) {
$tokens[] = /* ... */;
}
}
return$tokens;
}
}
class Lexer
{
publicfunctionlexify(array$tokens): array
{
$ast = [];
foreach ($tokensas$token) {
$ast[] = /* ... */;
}
return$ast;
}
}
class BetterJSAlternative
{
private$tokenizer;
private$lexer;
publicfunction__construct(Tokenizer$tokenizer, Lexer$lexer)
{
$this->tokenizer = $tokenizer;
$this->lexer = $lexer;
}
publicfunctionparse(string$code): void
{
$tokens = $this->tokenizer->tokenize($code);
$ast = $this->lexer->lexify($tokens);
foreach ($astas$node) {
// parse...
}
}
}แฟล็กเป็นตัวบอกผู้ใช้ของคุณว่าฟังก์ชันนี้ทำงานมากกว่าหนึ่งอย่าง ฟังก์ชันควรทำงานอย่างเดียว ให้แยกฟังก์ชันออกไป ถ้ามันต้องทำงานแยกเส้นทางไปตามค่าของบูลีน (boolean: True - False)
ไม่ดี:
functioncreateFile(string$name, bool$temp = false): void
{
if ($temp) {
touch('./temp/'.$name);
} else {
touch($name);
}
}ดี:
functioncreateFile(string$name): void
{
touch($name);
}
functioncreateTempFile(string$name): void
{
touch('./temp/'.$name);
}ฟังก์ชันจะทำให้เกิดผลข้างเคียงหากมันทำอะไรมากกว่าการรับค่าเข้ามาและคืนค่าออกไป ผลข้างเคียงอย่างเช่นเขียนลงไฟล์ แก้ไขตัวแปรระดับโกลบอล หรือโอนเงินของคุณไปให้คนแปลกหน้าโดยไม่ได้ตั้งใจ
เอาหละ บางทีคุณอาจจะต้องการผลข้างเคียงนั่นจริง ๆ อย่างในตัวอย่างข้างต้น คุณอาจต้องการที่จะเขียนลงไฟล์ สิ่งที่คุณต้องทำคือจับมันไปรวมไว้ส่วนกลาง อย่ามีฟังก์ชันหรือคลาสหลายตัวที่เขียนไฟล์ ให้มีเซอร์วิสเดียวที่ทำ ตัวเดียวเลย
เหตุผลหลักคือการหลีกเลี่ยงข้อผิดพลาดที่เจอบ่อย เช่น การแชร์สถานะ (state) ระหว่างอ๊อบเจคโดยที่ไม่มีโครงสร้างอะไรรองรับ การใช้ชนิตข้อมูลที่เปลี่ยนแปลงได้ (mutable data types) ที่มักจะถูกทำให้เปลี่ยนแปลงไปโดยโค้ดส่วนไหนก็ได้ และการไม่มีส่วนกลางมาควบคุมผลข้างเคียง ถ้าคุณจัดการเรื่องเหล่านี้ได้ คุณจะมีความสุขมากกว่าโปรแกรมเมอร์ส่วนใหญ่ทั่วโลกเลยทีเดียว
ไม่ดี:
// ตัวแปรระดับโกลบอล เรียกใช้แบบ reference ด้วยฟังก์ชันนี้// ถ้าเรามีฟังก์ชันที่ใช้ตัวแปรชื่อนี้เหมือนกัน ที่นีกลายเป็น array ไปแล้ว และจะเกิดข้อผิดพลาดได้$name = 'Ryan McDermott';
functionsplitIntoFirstAndLastName(): void
{
global$name;
$name = explode('', $name);
}
splitIntoFirstAndLastName();
var_dump($name); // ['Ryan', 'McDermott'];ดี:
functionsplitIntoFirstAndLastName(string$name): array
{
returnexplode('', $name);
}
$name = 'Ryan McDermott';
$newName = splitIntoFirstAndLastName($name);
var_dump($name); // 'Ryan McDermott';var_dump($newName); // ['Ryan', 'McDermott'];การทำให้ระดับโกลบอลมีมลพิษถือเป็นสิ่งที่ต้องหลีกเลี่ยงในหลาย ๆ ภาษา เพราะคุณอาจจะไปชนกับไลบรารี่อื่นและผู้ใช้งาน API ของคุณอาจจะไม่รู้มาก่อนจนกระทั้งไปเจอข้อผิดพลาดในระยะโปรดักชัน ลองคิดตามตัวอย่างนี้ จะเป็นอย่างไรถ้าคุณต้องการค่าคอนฟิกกุเรชันที่เป็นอาร์เรย์ คุณอาจจะเขียนฟังก์ชันแบบโกลบอลเช่น config() แต่มันไปชนกับฟังก์ชันที่อยู่ในไลบรารี่อื่นที่พยายามจะทำสิ่งเดียวกัน
ไม่ดี:
functionconfig(): array
{
return [
'foo' => 'bar',
]
}ดี:
class Configuration
{
private$configuration = [];
publicfunction__construct(array$configuration)
{
$this->configuration = $configuration;
}
publicfunctionget(string$key): ?string
{
returnisset($this->configuration[$key]) ? $this->configuration[$key] : null;
}
}โหลดคอนฟิกกุเรชันและสร้างอินสแตนส์ของคลาส Configuration
$configuration = newConfiguration([
'foo' => 'bar',
]);และต่อจากนี้คุณต้องใช้อินสแตนส์ของ Configuration นี้ในโปรแกรมของคุณทั้งหมด
ซิงเกิลตันเป็น แอนติ-แพทเทิน. ถอดความจาก Brian Button:
- มันมักจะถูกใช้เป็น อินแสตนส์แบบโกลบอล ทำไมมันถึงได้แย่นัก เพราะคุณ ซ่อนดีเพนเดนซี่ (Dependencies) ของโปรแกรมคุณแทนที่จะเปิดเผยมันผ่านอินเตอร์เฟส การจัดบางสิ่งให้อยู่ระดับโกลบอลเพื่อเลี่ยงการส่งผ่านมันไปมาถือว่าเป็น code smell.
- มันเป็นการละเมิด หลักการความรับผิดชอบเดียว (single responsibility principle): จากข้อเท็จจริงที่ว่า มันควบคุมการสร้างตัวเองและวงจรชีวิต (they control their own creation and lifecycle).
- มันทำให้เกิดโค้ดที่ยึดโยงกัน coupled. ทำการการทดสอบด้วยการเฟคทำได้ยากในหลายกรณี
- มันแบกสถานะไปด้วยตลอดอายุการทำงานของแอพลิเคชัน เป็นการขัดขวางการทดสอบอีกอย่าง ทำให้ต้องทดสอบแบบเป็นลำดับขั้น ซึ่งไม่ดีเลยกับการทำยูนิตเทส ทำไมเหรอ เพราะยูนิตเทสควรจะเป็นอิสระต่อกันไง
อันนี้เป็นความคิดเห็นที่ดีโดย Misko Hevery เกี่ยวกับ ต้นตอของปัญหา (root of problem).
ไม่ดี:
class DBConnection
{
privatestatic$instance;
privatefunction__construct(string$dsn)
{
// ...
}
publicstaticfunctiongetInstance(): DBConnection
{
if (self::$instance === null) {
self::$instance = newself();
}
returnself::$instance;
}
// ...
}
$singleton = DBConnection::getInstance();ดี:
class DBConnection
{
publicfunction__construct(string$dsn)
{
// ...
}
// ...
}สร้างอินสแตนส์ของคลาส DBConnection และคอนฟิกมันด้วย DSN.
$connection = newDBConnection($dsn);และคุณควรใช้อินสแตนส์ของ DBConnection ตลอดทั้งแอพลิเคชัน
ไม่ดี:
if ($article->state === 'published') {
// ...
}ดี:
if ($article->isPublished()) {
// ...
}ไม่ดี:
functionisDOMNodeNotPresent(\DOMNode$node): bool
{
// ...
}
if (!isDOMNodeNotPresent($node))
{
// ...
}ดี:
functionisDOMNodePresent(\DOMNode$node): bool
{
// ...
}
if (isDOMNodePresent($node)) {
// ...
}มันดูเป็นเรื่องเหนือจริงไปหน่อย ถ้าได้ยินครั้งแรกส่วนใหญ่จะพูดขึ้นว่า "จะทำงานอะไรได้ยังไงถ้าไม่มี if?"
คำตอบคือคุณสามารถใช้หลักการโพลีมอฟิสมาแทนได้ในหลาย ๆ กรณี คำถามต่อมาคือ "อืม ก็ฟังดูดีนะ แต่ทำไมฉันต้องทำงั้นด้วยล่ะ?"
คำตอบก็คือแนวคิดคลีนโค้ดที่เราเรียนผ่านมาแล้ว นั่นคือ ฟังก์ชันควรทำงานอย่างเดียว เมื่อคุณมีคลาสและฟังก์ชันที่มี if แสดงว่าคุณกำลังบอกผู้ใช้ว่าฟังก์ชันของคุณกำลังทำงานมากกว่าอย่างเดียว จำได้มั๊ย? เพราะงั้นทำอย่างเดียวพอ
ไม่ดี:
class Airplane
{
// ...publicfunctiongetCruisingAltitude(): int
{
switch ($this->type) {
case'777':
return$this->getMaxAltitude() - $this->getPassengerCount();
case'Air Force One':
return$this->getMaxAltitude();
case'Cessna':
return$this->getMaxAltitude() - $this->getFuelExpenditure();
}
}
}ดี:
interface Airplane
{
// ...publicfunctiongetCruisingAltitude(): int;
}
class Boeing777 implements Airplane
{
// ...publicfunctiongetCruisingAltitude(): int
{
return$this->getMaxAltitude() - $this->getPassengerCount();
}
}
class AirForceOne implements Airplane
{
// ...publicfunctiongetCruisingAltitude(): int
{
return$this->getMaxAltitude();
}
}
class Cessna implements Airplane
{
// ...publicfunctiongetCruisingAltitude(): int
{
return$this->getMaxAltitude() - $this->getFuelExpenditure();
}
}PHP เป็นภาษาที่ไม่จำเป็นต้องประกาศชนิตของตัวแปร ซึ่งหมายความว่าฟังก์ชันของคุณสามารถรับอาร์กิวเมนท์เป็นอะไรก็ได้ บางทีก็โดนความเสรีนี้แว้งกัดบ่อย ๆ และมันก็ยั่วใจเหลือเกินที่จะทำการตรวจสอบชนิดข้อมูลก่อนทำงานในฟังก์ชัน มีหลายวิธีที่จะหลีกเลี่ยงได้ อย่างแรกที่ควรพิจารณาคือความสอดคล้องของ API*
ไม่ดี:
functiontravelToTexas($vehicle): void
{
if ($vehicleinstanceof Bicycle) {
$vehicle->pedalTo(newLocation('texas'));
} elseif ($vehicleinstanceof Car) {
$vehicle->driveTo(newLocation('texas'));
}
}ดี:
functiontravelToTexas(Traveler$vehicle): void
{
$vehicle->travelTo(newLocation('texas'));
}*ผู้แปล: กรณีนี้คือการใช้ Type Hinting ใน PHP 5 หรือ Type Declaration ใน PHP 7 ถ้าตัวแปรที่ส่งเข้าฟังก์ชันไม่ใช่ชนิดข้อมูลที่ระบุก็จะฟ้องข้อผิดพลาดออกมา
ถ้าคุณทำงานกับชนิดตัวแปรพื้นฐานอย่าง สตริง อินทิเจอ (integer) หรืออาร์เรย์ และคุณใช้ PHP 7+ และคุณไม่สามารถใช้โพลีมอฟิสได้ แต่คุณยังรู้สึกว่าต้องทำการตรวจสอบชนิดข้อมูลอยู่ดี คุณควรพิจารณาเรื่อง type declaration หรือ โหมดเข้มงวด (strict mode) มันจะกำหนดให้คุณต้องประกาศตัวแปรพร้อมระบุชนิดข้อมูลด้วย ปัญหาของการตรวจสอบชนิดข้อมูลด้วยตนเองคือมันต้องเพิ่มโค้ดจำนวนมากเข้าไปเพื่อให้แน่ใจว่าได้ชนิดข้อมูลที่ถูกต้อง ซึ่งไม่คุ้มกับที่ต้องเสียโค้ดที่อ่านง่ายไป พยายามให้โค้ดสะอาด เขียนเทสดี ๆ และมีการรีวิวโค้ดอย่างมีประสิทธิภาพ หรือไม่อย่างนั้นก็ทำทุกอย่างและใช้การกำหนดชนิดข้อมูลหรือใช้โหมดเข้มงวด
ไม่ดี:
functioncombine($val1, $val2): int
{
if (!is_numeric($val1) || !is_numeric($val2)) {
thrownew \Exception('Must be of type Number');
}
return$val1 + $val2;
}ดี:
functioncombine(int$val1, int$val2): int
{
return$val1 + $val2;
}โค้ดที่ไม่ได้ใช้แล้วก็ไม่ดีพอ ๆ กับโค้ดที่ซ้ำ ไม่มีเหตุผลอะไรจะเก็บไว้ในงาน ถ้ามันไม่ได้ถูกใช้ก็เอาออกไปเลย ยังไงมันก็ยังอยู่ในระบบเก็บเวอร์ชัน (version control system) อยู่ดีเมื่อคุณต้องการ
ไม่ดี:
functionoldRequestModule(string$url): void
{
// ...
}
functionnewRequestModule(string$url): void
{
// ...
}
$request = newRequestModule($requestUrl);
inventoryTracker('apples', $request, 'www.inventory-awesome.io');ดี:
functionrequestModule(string$url): void
{
// ...
}
$request = requestModule($requestUrl);
inventoryTracker('apples', $request, 'www.inventory-awesome.io');ใน PHP คุณสามารถใช้คีย์เวิร์ด public, protected และ private กับเมทธอด
ใช้มันซะ คุณจะได้ควบคุมการเปลี่ยนแปลงพร็อบเพอตีของอ็อบเจคได้
- เมื่อคุณต้องการจะทำอะไรมากกว่าดึงข้อมูลพร็อบเพอตี คุณจะไม่ต้องค้นหาและเปลี่ยน แอกเซสเซอร์ (accessor) ทุกตัวในโค้ดเบส
- ทำให้เพิ่มการตรวจสอบได้ง่ายเมื่อต้องใช้
set - ซ่อนเนื้อหาไว้ภายใน
- ง่ายที่จะเพิ่มการทำบันทึก (logging) และการจัดการข้อผิดพลาดเมื่อทำการดึงและเปลี่ยนข้อมูล
- เมื่อสืบทอดคลาสนี้ คุณสามารถจะเปลี่ยนการทำงานจากแบบเดิมได้
- คุณสามารถทำเลซี่โหลด (lazy load) พร็อบเพอตีของอ็อบเจคของคุณได้ เช่นโหลดจาก server
ยิ่งกว่านั้น อันนี้เป็นส่วนหนึ่งของหลักการ โอเพน-โคลส (Open/Closed principle)
ไม่ดี:
class BankAccount
{
public$balance = 1000;
}
$bankAccount = newBankAccount();
// ซื้อรองเท้า...$bankAccount->balance -= 100;ดี:
class BankAccount
{
private$balance;
publicfunction__construct(int$balance = 1000)
{
$this->balance = $balance;
}
publicfunctionwithdraw(int$amount): void
{
if ($amount > $this->balance) {
thrownew \Exception('Amount greater than available balance.');
}
$this->balance -= $amount;
}
publicfunctiondeposit(int$amount): void
{
$this->balance += $amount;
}
publicfunctiongetBalance(): int
{
return$this->balance;
}
}
$bankAccount = newBankAccount();
// ซื้อรองเท้า ...$bankAccount->withdraw($shoesPrice);
// เช็คยอด$balance = $bankAccount->getBalance();- เมดธอดและพร็อบเพอตีที่เป็น
publicเสี่ยงต่อการถูกเปลี่ยนแปลงมาก เพราะโค้ดอื่นบางตัวอาจจะมาอิงอยู่กับพวกมัน และคุณไม่สามารถควบคุมได้ว่าโค้ดส่วนไหนที่ถูกยึดโยงอยู่ การเปลี่ยนแปลงใด ๆ ในคลาสจะทำให้เกิดอันตรายกับผู้ใช้คลาสทั้งหมด - โมดิฟายเออร์
protectedก็อันตรายไม่แพ้ public เพราะมันถูกเข้าถึงได้จากคลาสลูกคลาสไหนก็ได้ นั่นหมายความว่าสิ่งที่แตกต่างระหว่าง public กับ protected ก็แค่วิธีการเข้าถึงเท่านั้นเอง แต่การเอนแคปซูเลชั่นจะรับประกันว่าทุกอย่างจะเหมือนเดิม การเปลี่ยนแปลงใด ๆ ในคลาสจะทำให้เกิดอันตรายกับคลาสลูกทั้งหมด - โมดิฟายเออร์
privateรับประกันว่าโค้ดนั้น ความเสี่ยงในการเปลี่ยนแปลงโค้ดจะถูกจำกัดอยู่ในคลาสเดียวเท่านั้น (คุณจะปลอดภัยที่จะเปลี่ยนอะไรก็ได้และไม่ต้องเจอกับ เจงก้า เอฟเฟค (Jenga effect))
เพราะฉะนั้น ใช้ private เป็นค่าเริ่มต้น และใช้ public/protected เมื่อคุณต้องการให้คลาสอื่นเข้าถึงได้
คุณสามารถอ่านข้อมูลเพิ่มเติมได้จาก บล็อกโพสนี้ เขียนโดย Fabien Potencier
ไม่ดี:
class Employee
{
public$name;
publicfunction__construct(string$name)
{
$this->name = $name;
}
}
$employee = newEmployee('John Doe');
echo'Employee name: '.$employee->name; // Employee name: John Doeดี:
class Employee
{
private$name;
publicfunction__construct(string$name)
{
$this->name = $name;
}
publicfunctiongetName(): string
{
return$this->name;
}
}
$employee = newEmployee('John Doe');
echo'Employee name: '.$employee->getName(); // Employee name: John Doeอย่างที่กล่าวถึงใน Design Patterns โดยแก็งสี่สหาย (Gang of Four) คุณควรจะใช้การประกอบมากกว่าการสืบทอดคลาสในจุดที่ทำได้ มีเหตุผลดี ๆ มากมายที่จะใช้การสืบทอดคลาส และมันก็มีเหตุผลอีกมากมายเหมือนกันที่จะใช้การประกอบ จุดหลักของคำกล่าวนี้คือถ้าใจคุณมุ่งไปที่การสืบทอด พยายาม ลองคิดว่าการประกอบจะแก้ปัญหาให้คุณได้ดีกว่า ซึ่งในบางกรณีมันเป็นเช่นนั้น
คุณอาจจะกำลังคิดว่าแล้วเมื่อไรฉันถึงควรจะใช้การสืบทอดล่ะ? มันขึ้นอยู่กับปัญหาที่คุณเจอ แต่ปัญหาเหล่านั้นมันก็มีไม่กี่อย่างที่การสืบทอดจะดูมีเหตุผลกว่าการประกอบ
- การสืบทอดนั้นแสดงถึงความสัมพันธ์แบบการ "เป็นสิ่งหนึ่ง" และไม่ได้เป็นแบบ "มีสิ่งหนึ่ง" (คน -> สัตว์ กับ ผู้ใช้ -> รายละเอียดผู้ใช้)
- คุณสามารถใช้โค้ดเก่าจากคลาสแม่ (คนสามารถเคลื่อนที่ได้เหมือนสัตว์ทั่วไป)
- คุณต้องการจะให้เกิดการเปลี่ยนแปลงระดับโกลบอลโดยการเปลี่ยนที่คลาสแม่ (เปลี่ยนอัตราการเผาผลาญแคลอรี่ของสัตว์เมื่อเคลื่อนที่)
ไม่ดี:
class Employee {
private$name;
private$email;
publicfunction__construct(string$name, string$email)
{
$this->name = $name;
$this->email = $email;
}
// ...
}
// ไม่ดี เพราะ Employee "มี" ข้อมูลการเสียภาษี// Employee TaxData ไม่ได้เป็น Employeeclass EmployeeTaxData extends Employee {
private$ssn;
private$salary;
publicfunction__construct(string$name, string$email, string$ssn, string$salary)
{
parent::__construct($name, $email);
$this->ssn = $ssn;
$this->salary = $salary;
}
// ...
}ดี:
class EmployeeTaxData {
private$ssn;
private$salary;
publicfunction__construct(string$ssn, string$salary)
{
$this->ssn = $ssn;
$this->salary = $salary;
}
// ...
}
class Employee {
private$name;
private$email;
private$taxData;
publicfunction__construct(string$name, string$email)
{
$this->name = $name;
$this->email = $email;
}
publicfunctionsetTaxData(string$ssn, string$salary)
{
$this->taxData = newEmployeeTaxData($ssn, $salary);
}
// ...
}ฟลูเอนท์ อินเตอร์เฟส เป็นลักษณะการโปรแกรมที่มีเป้าหมายเพื่อให้ซอสโค้ดอ่านได้ง่ายโดย การเรียกเมธอดเป็นลูกโซ่
ข้อดีของมันคือในบางกรณีแพทเทินนี้จะลดรูปของโค้ดลง ตัวอย่างเช่น PHPUnit Mock Builder หรือกับ Doctrine Query Builder) แต่บ่อยครั้งที่มันมีต้นทุนสูง
- ละเมิด Encapsulation
- ละเมิด Decorators
- ทดสอบแบบ mock ได้ยาก
- ทำให้อ่านสิ่งที่เปลี่ยนแปลง (diffs) ในโค้ดได้ยาก
คุณสามารถอ่านข้อมูลเพิ่มเติมในเรื่องนี้ได้จาก บล็อกโพส เขียนโดย Marco Pivetta
ไม่ดี:
class Car
{
private$make = 'Honda';
private$model = 'Accord';
private$color = 'white';
publicfunctionsetMake(string$make): self
{
$this->make = $make;
// NOTE: Returning this for chainingreturn$this;
}
publicfunctionsetModel(string$model): self
{
$this->model = $model;
// NOTE: Returning this for chainingreturn$this;
}
publicfunctionsetColor(string$color): self
{
$this->color = $color;
// NOTE: Returning this for chainingreturn$this;
}
publicfunctiondump(): void
{
var_dump($this->make, $this->model, $this->color);
}
}
$car = (newCar())
->setColor('pink')
->setMake('Ford')
->setModel('F-150')
->dump();ดี:
class Car
{
private$make = 'Honda';
private$model = 'Accord';
private$color = 'white';
publicfunctionsetMake(string$make): void
{
$this->make = $make;
}
publicfunctionsetModel(string$model): void
{
$this->model = $model;
}
publicfunctionsetColor(string$color): void
{
$this->color = $color;
}
publicfunctiondump(): void
{
var_dump($this->make, $this->model, $this->color);
}
}
$car = newCar();
$car->setColor('pink');
$car->setMake('Ford');
$car->setModel('F-150');
$car->dump();The final should be used whenever possible:
- It prevents uncontrolled inheritance chain.
- It encourages composition.
- It encourages the Single Responsibility Pattern.
- It encourages developers to use your public methods instead of extending the class to get access on protected ones.
- It allows you to change your code without any break of applications that use your class.
The only condition is that your class should implement an interface and no other public methods are defined.
For more informations you can read the blog post on this topic written by Marco Pivetta (Ocramius).
Bad:
finalclass Car
{
private$color;
publicfunction__construct($color)
{
$this->color = $color;
}
/** * @return string The color of the vehicle */publicfunctiongetColor() {
return$this->color;
}
}Good:
interface Vehicle
{
/** * @return string The color of the vehicle */publicfunctiongetColor();
}
finalclass Car implements Vehicle
{
private$color;
publicfunction__construct($color)
{
$this->color = $color;
}
/** * {@inheritdoc} */publicfunctiongetColor() {
return$this->color;
}
}SOLID is the mnemonic acronym introduced by Michael Feathers for the first five principles named by Robert Martin, which meant five basic principles of object-oriented programming and design.
- S: Single Responsibility Principle (SRP)
- O: Open/Closed Principle (OCP)
- L: Liskov Substitution Principle (LSP)
- I: Interface Segregation Principle (ISP)
- D: Dependency Inversion Principle (DIP)
As stated in Clean Code, "There should never be more than one reason for a class to change". It's tempting to jam-pack a class with a lot of functionality, like when you can only take one suitcase on your flight. The issue with this is that your class won't be conceptually cohesive and it will give it many reasons to change. Minimizing the amount of times you need to change a class is important. It's important because if too much functionality is in one class and you modify a piece of it, it can be difficult to understand how that will affect other dependent modules in your codebase.
Bad:
class UserSettings
{
private$user;
publicfunction__construct(User$user)
{
$this->user = $user;
}
publicfunctionchangeSettings(array$settings): void
{
if ($this->verifyCredentials()) {
// ...
}
}
privatefunctionverifyCredentials(): bool
{
// ...
}
}Good:
class UserAuth {
private$user;
publicfunction__construct(User$user)
{
$this->user = $user;
}
publicfunctionverifyCredentials(): bool
{
// ...
}
}
class UserSettings {
private$user;
private$auth;
publicfunction__construct(User$user) {
$this->user = $user;
$this->auth = newUserAuth($user);
}
publicfunctionchangeSettings(array$settings): void
{
if ($this->auth->verifyCredentials()) {
// ...
}
}
}As stated by Bertrand Meyer, "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification." What does that mean though? This principle basically states that you should allow users to add new functionalities without changing existing code.
Bad:
abstractclass Adapter
{
protected$name;
publicfunctiongetName(): string
{
return$this->name;
}
}
class AjaxAdapter extends Adapter
{
publicfunction__construct()
{
parent::__construct();
$this->name = 'ajaxAdapter';
}
}
class NodeAdapter extends Adapter
{
publicfunction__construct()
{
parent::__construct();
$this->name = 'nodeAdapter';
}
}
class HttpRequester
{
private$adapter;
publicfunction__construct(Adapter$adapter)
{
$this->adapter = $adapter;
}
publicfunctionfetch(string$url): Promise
{
$adapterName = $this->adapter->getName();
if ($adapterName === 'ajaxAdapter') {
return$this->makeAjaxCall($url);
} elseif ($adapterName === 'httpNodeAdapter') {
return$this->makeHttpCall($url);
}
}
privatefunctionmakeAjaxCall(string$url): Promise
{
// request and return promise
}
privatefunctionmakeHttpCall(string$url): Promise
{
// request and return promise
}
}Good:
interface Adapter
{
publicfunctionrequest(string$url): Promise;
}
class AjaxAdapter implements Adapter
{
publicfunctionrequest(string$url): Promise
{
// request and return promise
}
}
class NodeAdapter implements Adapter
{
publicfunctionrequest(string$url): Promise
{
// request and return promise
}
}
class HttpRequester
{
private$adapter;
publicfunction__construct(Adapter$adapter)
{
$this->adapter = $adapter;
}
publicfunctionfetch(string$url): Promise
{
return$this->adapter->request($url);
}
}This is a scary term for a very simple concept. It's formally defined as "If S is a subtype of T, then objects of type T may be replaced with objects of type S (i.e., objects of type S may substitute objects of type T) without altering any of the desirable properties of that program (correctness, task performed, etc.)." That's an even scarier definition.
The best explanation for this is if you have a parent class and a child class, then the base class and child class can be used interchangeably without getting incorrect results. This might still be confusing, so let's take a look at the classic Square-Rectangle example. Mathematically, a square is a rectangle, but if you model it using the "is-a" relationship via inheritance, you quickly get into trouble.
Bad:
class Rectangle
{
protected$width = 0;
protected$height = 0;
publicfunctionsetWidth(int$width): void
{
$this->width = $width;
}
publicfunctionsetHeight(int$height): void
{
$this->height = $height;
}
publicfunctiongetArea(): int
{
return$this->width * $this->height;
}
}
class Square extends Rectangle
{
publicfunctionsetWidth(int$width): void
{
$this->width = $this->height = $width;
}
publicfunctionsetHeight(int$height): void
{
$this->width = $this->height = $height;
}
}
functionprintArea(Rectangle$rectangle): void
{
$rectangle->setWidth(4);
$rectangle->setHeight(5);
// BAD: Will return 25 for Square. Should be 20.echosprintf('%s has area %d.', get_class($rectangle), $rectangle->getArea()).PHP_EOL;
}
$rectangles = [newRectangle(), newSquare()];
foreach ($rectanglesas$rectangle) {
printArea($rectangle);
}Good:
The best way is separate the quadrangles and allocation of a more general subtype for both shapes.
Despite the apparent similarity of the square and the rectangle, they are different. A square has much in common with a rhombus, and a rectangle with a parallelogram, but they are not subtype. A square, a rectangle, a rhombus and a parallelogram are separate shapes with their own properties, albeit similar.
interface Shape
{
publicfunctiongetArea(): int;
}
class Rectangle implements Shape
{
private$width = 0;
private$height = 0;
publicfunction__construct(int$width, int$height)
{
$this->width = $width;
$this->height = $height;
}
publicfunctiongetArea(): int
{
return$this->width * $this->height;
}
}
class Square implements Shape
{
private$length = 0;
publicfunction__construct(int$length)
{
$this->length = $length;
}
publicfunctiongetArea(): int
{
return$this->length ** 2;
}
}
functionprintArea(Shape$shape): void
{
echosprintf('%s has area %d.', get_class($shape), $shape->getArea()).PHP_EOL;
}
$shapes = [newRectangle(4, 5), newSquare(5)];
foreach ($shapesas$shape) {
printArea($shape);
}ISP states that "Clients should not be forced to depend upon interfaces that they do not use."
A good example to look at that demonstrates this principle is for classes that require large settings objects. Not requiring clients to set up huge amounts of options is beneficial, because most of the time they won't need all of the settings. Making them optional helps prevent having a "fat interface".
Bad:
interface Employee
{
publicfunctionwork(): void;
publicfunctioneat(): void;
}
class Human implements Employee
{
publicfunctionwork(): void
{
// ....working
}
publicfunctioneat(): void
{
// ...... eating in lunch break
}
}
class Robot implements Employee
{
publicfunctionwork(): void
{
//.... working much more
}
publicfunctioneat(): void
{
//.... robot can't eat, but it must implement this method
}
}Good:
Not every worker is an employee, but every employee is a worker.
interface Workable
{
publicfunctionwork(): void;
}
interface Feedable
{
publicfunctioneat(): void;
}
interface Employee extends Feedable, Workable
{
}
class Human implements Employee
{
publicfunctionwork(): void
{
// ....working
}
publicfunctioneat(): void
{
//.... eating in lunch break
}
}
// robot can only workclass Robot implements Workable
{
publicfunctionwork(): void
{
// ....working
}
}This principle states two essential things:
- High-level modules should not depend on low-level modules. Both should depend on abstractions.
- Abstractions should not depend upon details. Details should depend on abstractions.
This can be hard to understand at first, but if you've worked with PHP frameworks (like Symfony), you've seen an implementation of this principle in the form of Dependency Injection (DI). While they are not identical concepts, DIP keeps high-level modules from knowing the details of its low-level modules and setting them up. It can accomplish this through DI. A huge benefit of this is that it reduces the coupling between modules. Coupling is a very bad development pattern because it makes your code hard to refactor.
Bad:
class Employee
{
publicfunctionwork(): void
{
// ....working
}
}
class Robot extends Employee
{
publicfunctionwork(): void
{
//.... working much more
}
}
class Manager
{
private$employee;
publicfunction__construct(Employee$employee)
{
$this->employee = $employee;
}
publicfunctionmanage(): void
{
$this->employee->work();
}
}Good:
interface Employee
{
publicfunctionwork(): void;
}
class Human implements Employee
{
publicfunctionwork(): void
{
// ....working
}
}
class Robot implements Employee
{
publicfunctionwork(): void
{
//.... working much more
}
}
class Manager
{
private$employee;
publicfunction__construct(Employee$employee)
{
$this->employee = $employee;
}
publicfunctionmanage(): void
{
$this->employee->work();
}
}Try to observe the DRY principle.
Do your absolute best to avoid duplicate code. Duplicate code is bad because it means that there's more than one place to alter something if you need to change some logic.
Imagine if you run a restaurant and you keep track of your inventory: all your tomatoes, onions, garlic, spices, etc. If you have multiple lists that you keep this on, then all have to be updated when you serve a dish with tomatoes in them. If you only have one list, there's only one place to update!
Often you have duplicate code because you have two or more slightly different things, that share a lot in common, but their differences force you to have two or more separate functions that do much of the same things. Removing duplicate code means creating an abstraction that can handle this set of different things with just one function/module/class.
Getting the abstraction right is critical, that's why you should follow the SOLID principles laid out in the Classes section. Bad abstractions can be worse than duplicate code, so be careful! Having said this, if you can make a good abstraction, do it! Don't repeat yourself, otherwise you'll find yourself updating multiple places any time you want to change one thing.
Bad:
functionshowDeveloperList(array$developers): void
{
foreach ($developersas$developer) {
$expectedSalary = $developer->calculateExpectedSalary();
$experience = $developer->getExperience();
$githubLink = $developer->getGithubLink();
$data = [
$expectedSalary,
$experience,
$githubLink
];
render($data);
}
}
functionshowManagerList(array$managers): void
{
foreach ($managersas$manager) {
$expectedSalary = $manager->calculateExpectedSalary();
$experience = $manager->getExperience();
$githubLink = $manager->getGithubLink();
$data = [
$expectedSalary,
$experience,
$githubLink
];
render($data);
}
}ดี:
functionshowList(array$employees): void
{
foreach ($employeesas$employee) {
$expectedSalary = $employee->calculateExpectedSalary();
$experience = $employee->getExperience();
$githubLink = $employee->getGithubLink();
$data = [
$expectedSalary,
$experience,
$githubLink
];
render($data);
}
}ดีมาก:
ใช้โค้ดที่สั้นกระชับเป็นการดีที่สุด
functionshowList(array$employees): void
{
foreach ($employeesas$employee) {
render([
$employee->calculateExpectedSalary(),
$employee->getExperience(),
$employee->getGithubLink()
]);
}
}บทความนี้มีในภาษาอื่นด้วย:
- 🇨🇳 Chinese:
- 🇷🇺 Russian:
- 🇪🇸 Spanish:
- 🇧🇷 Portuguese:
- 🇹🇭 Thai:
- 🇫🇷 French:
- 🇻🇳 Vietnamese
- 🇰🇷 Korean: