Loading...

2019-01-05

Home » » Những lỗi nên tránh của lập trình viên PHP

Những lỗi nên tránh của lập trình viên PHP

19:13:00    

Lỗi cơ bản về empty vs isset

Mức độ phổ biến: Rất phổ biến
Mức độ nghiêm trọng: Không quá nghiêm trọng, nhưng mà nó làm nhức đầu hại não nhưng thằng dev khác sau này khi đọc code của bạn.
Lỗi vè sử dụng các hàm isset cho các đối tượng được lồng vô nhau, lỗi này phổ biến đến mất mình thấy nó có mặt khắp mọi nơi ở hầu hết các công ty mà mình từng làm việc.
Vấn đề này không phải là sai, nhưng nó làm cho code trể nên rối rắm, dài dòng không cần thiết.
################### KHÔNG NÊN ###################
if (isset($product['shop'] && isset($product['shop']['id']) {
  doSomeThing($product['shop']);
}

So sánh với:

################### NÊN ###################
if (!empty($product['shop']['id']) {
  doSomeThing($product['shop']);
}

Lời bàn:

Tại sao lập trình viên lại gặp lỗi này:
  1. Là docopy & paste, tại thấy thằng khác trước nó code vậy, nên mình copy và paste vậy luôn.
  2. Chưa bao giờ bạn thử hàmemptynó hoạt động thế nào, và rất ngại khi sửa lại cái hàm đó, bởi vì mấy thằng lead nó nói, khi code hoạt động ổn định thì đừng có sửa, ai sẽ review.
Làm thế nào để không gặp những lỗi này:
  1. Đừng bao giờcopy & pastecode của người khác mà không suy nghĩ.
  2. Và cũng đừng bao giờ code mà không suy nghĩ là các hàm như vậy khác nhau như thế nào.

Lỗi cộng chuỗi

Mức độ phổ biến: Rất phổ biến
Mức độ nghiêm trọng: Lỗi này là một trong những nguyên nhân hàng đầu gây ra bug, khó bảo trì và phát triển thêm tính năng.
Xem xét một ví dụ, build một cái chuỗi url:
# ################## KHÔNG NÊN ##############
# Giả sử ta có một array params như sau:
$params = [
  'a' => 1,
  'b' => 2,
  'c' => 3
];
# Bạn muốn build thành chuỗi:
# url = //localhost?a=1&b=2&c=3
$query_string = [];
for($params as $key => $value){
  $query_string[] = $key.'='.$value;
}
$url = '//localhost/?'.implode('&', $query_string);

So sánh với:

# ################## NÊN ##############
# Giả sử ta có một array params như sau:
$params = [
  'a' => 1,
  'b' => 2,
  'c' => 3
];
# Bạn muốn build thành chuỗi:
# url = //localhost?a=1&b=2&c=3
$url = '//localhost/?'.http_build_query($params);
Có thể nói cái cách làm trên không chỉ là dài dòng và không cần thiết, mà còn tìm ẩn nhiều rủi ro cơ bản, chẳng hạn khi các$paramsphức tạp ở dạng array, hay là các chuỗi cần dược encode.
PHP cung cấp cho chúng ta rất rất nhiều hàm để xử lý chuỗi, bạn cần phải đọc tài liệu và viết đúng những hàm mà mình cần.
Chưa hết, khi bạn sử dụng Framework bạn cần phải sử dụng hàm của nó để build ra url từ router thì ứng dụng mới hoạt động đúng.

Lời bàn:

Tại sao lập trình viên lại gặp lỗi này:
  1. Không đọc tài liệu của PHP để viết cho đúng best practice.
  2. Không đọc Framework của mình có gì và hiểu đúng Framework.
Làm thế nào để không gặp những lỗi này:
  1. Khi bạn gặp phải một hàm gì đó quá phức tạp hay nghĩ ra một cách làm gì đó phức tạp thì bạn phải tin rằng mình đang làm sai gì đó và đâu đó đã có cách viết tốt hơn.
  2. Lập trình viên trên thế giới rất lười và phần lớn những điều mà bạn nghĩ bạn gặp đã được giải quyết, bạn chỉ cần search Google là sẽ ra cách giải quyết thôi.
  3. Nên đọc và hiểu Framework mà mình đang viết, các khái niệm nó stack với nhau.

Sử dụng các con số

Mức độ phổ biến: Đâu đâu cũng thấy
Mức độ nghiêm trọng: Giống như một mình bạn đang chống lại thế giới.
Như ví dụ bên dưới, bạn không nên so sánh với các giá trị như 2, 1, người khác đọc code sẽ không thể nào hiểu được các giá trị đó là gì.
################## KHÔNG NÊN ##############
if( $product->status === 2 && $product->stock_status === 1 ){
  // do something
}

So sánh với:

################## NÊN ##############
class Product {
  const STATUS_OK = 2;
  const STOCK_STATATUS_AVAILABLE = 1;
  public function canDoSomething() {
    if ($product->status === static::STATUS_OK
      && $product->stock_status === static::STOCK_STATATUS_AVAILABLE) {
      return true;
    }
    return false;
  }
}
if ($product->canDoSomething()) {
  // do something
}

Ket:

  1. Viết như cách KHÔNG NÊN rất rối rắm, và không thể tái sử dụng.
  2. Đóng gói function sẽ làm cho code dễ đọc, dễ bảo trì, fix bug và tái sử dụng hơn.

Lỗi cơ bản về if

Mức độ phổ biến: Rất phổ biến
Mức độ nghiêm trọng: Lỗi này không nghiêm trọng nhưng mà nó làm cho code đọc khó hiểu
Trong việc lập trình hàm if là các hàm không thể tránh khỏi, nhưng gần như là chúng ta sử dụng các hàm này một cách mù quán mà không hề suy nghĩ.
################## KHÔNG NÊN ##############
function doSomething($params = []) {
  $result = [];
  if( !empty($params) ) {
    $result['r1'] = $params['p1'] + $params['p2'];
    //do something 1
    // ...
    //do something n
    $result['r2'] = doAnother($params['p3'], $params['p4']);
  }
  return $result;
}

So sánh với:

################## NÊN ##############
function doSomething($params = []) {
  $result = [];

  if( empty($params) ) {
    return $result;
  }

  $result['r1'] = $params['p1'] + $params['p2'];
  //do something 1
  // ...
  //do something n
  $result['r2'] = doAnother($params['p3'], $params['p4']);

  return $result;
}
Mức độ nghiêm trọng của tư duy này là khi các hàm if lồng nhau, thì sẽ dẫn đến các hàm khó đọc, tốt nhất là các bạn nên khiểm tra điều kiện nếu các biến truyền vào empty thì nên return trước.

Ket:

Tại sao lập trình viên lại gặp lỗi này:
  1. Các lập trình viên hay nghĩ tới trường hợp đúng và luôn code cho trường hợp đúng trước hết.
Làm thế nào để không gặp những lỗi này:
  1. Bạn phải tư duy về những chức năng có thể gây lỗi trước.
  2. Tư duy nếu hàm các bạn viết ra cho người khác xài mà không được truyền vào đúng như mong đợi thì thế nào.

Lỗi về hàm if trong loop

Mức độ phổ biến:Rất phổ biến
Mức độ nghiêm trọng: Lỗi này làm chương trình rất khó đọc hiểu logic và rất dễ gây bug
Lỗi về nested của loop vàif/elsekhá phổ biến, và việc de-nested điều này không phải lúc nào cũng dễ, tuy nhiên không phải là không làm được.
Ví dụ bên dưới là việc sử dụng hàmbreakvàcontinueđể giải tránh code các hàm if lồng nhau:
################## KHÔNG NÊN ##############
function doSomething($an_array, $an_ignore_array) {
  $result = [];
  foreach ($an_array as $an_element) {
    if (count($result) < MAX_LIST) {
      if(!in_array($an_element['id'], $an_ignore_array)) {
        $result[$an_element['id']] = $an_element;
      }
    } else {
      break;
    }
  }
  return $result;
}

So sánh với:

################## NÊN ##############
function doSomething($an_array, $an_ignore_array) {
  $result = [];
  foreach ($an_array as $an_element) {
    if (count($result) >= MAX_LIST) {
      break;
    }
    if (in_array($an_element['id'], $an_ignore_array)) {
      continue;
    }
    $result[$an_element['id']] = $an_element;
  }
  return $result;
}

Thảm hoạ về dùng CURL

Mức độ phổ biến: Thỉnh thoảng có gặp
Mức độ nghiêm trọng: Nó làm code cũng khó hiểu và phức tạp không kém.
Hãy nhìn logic để thực hiện một request đến đâu đó với method POST.

################## KHÔNG NÊN ##############
function getSomething($url, $post_data = []) {
  $token = 'Token';
  $headers = array(
    "X-Token: $token",
  );

  $ch = curl_init();

  curl_setopt($ch, CURLOPT_URL, $url);
  curl_setopt($ch, CURLOPT_HTTPHEADER, $headers);
  curl_setopt($ch, CURLOPT_POST, 1);
  curl_setopt($ch, CURLOPT_POSTFIELDS, json_encode($post_data));
  curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);

  $response = [];
  $result   = curl_exec($ch);
  if($result) {
    $response = json_decode($result);
  }

  curl_close($ch);

  return $response;
}

So sánh với:

################## NÊN ##############
function getSomething($url, $post_data = []) {
  $token = 'Token';
  $headers = array(
    "X-Token: $token",
  );

  $client = new \GuzzleHttp\Client();
  $res = $client->post($url, $headers, $post_data);

  $response = [];
  $result = $res->getBody();
  if($result) {
    $response = json_decode($result);
  }
  return $response;
}
Làm ơn đừng sử dụng curl nó làm cho code rất khó đọc, rồi bạn thêm, xoá, sửa các vấn đề về proxy, sercurity v.v… các bug với curl, rồi với các php version khác nhau.
Hãy sử dụng các thư viện, function mà người khác đã code, đã làm, đã build, đừng code lại một hàm hay một tính năng khác, khi mà người khác đã làm ra nó.


Ket:

Tại sao lập trình viên lại gặp lỗi này:
  1. Hầu hết các bạn dev đều cho rằng sử dụng các hàmbuilt-inmà không sử dụng thư viện sẽ làm code chạy nhanh hơn.
  2. Hoặc là cho rằng tính năng quá đơn giản chỉ cần lạicopy & pastecode từ đâu đó, sửa sửa lại cho nó chạy là ngầu rồi.
Làm thế nào để không gặp những lỗi này:
  1. Chương trình chạy nhanh hay chậm không phải vì bạn có sử dụng thêm thử việc hay code ở dạng function built-in. Phần lớn ứng dụng chậm là vì các nguyên nhân liên quan tới xử lý dữ liệu, tính toán.
  2. Đừng bao giờ nghĩ là nó cần nhanh hơn một vài nano hoặc là milisecond giây làm gì khi mà nó có thể tìm ẩn rủi ro về code và chi phí phát triển ứng dụng.

Lỗi function wrapper

Mức độ phổ biến: Rất hay gặp
Mức độ nghiêm trọng: Nó làm code trở nên khó debug và thật sự chẳng để làm gì cả.
################## KHÔNG NÊN ##############
class Model extends AbstractModel{

  public function fetchOnce($array){
      return $this->findOneBy($array);
  }
}
Thật sự là đừng bao giờ làm như vậy cả, bạn không nên wrap một function để rename nó cho bạn dễ đọc và dễ hiểu, điều bạn cần là bạn hiểu cái framework mà bạn đang xài, điều đó dễ dàng cho bạn nhớ keyword và có khái niệm chung với mọi người.
Việc tạo ra một function khác không chỉ làm cho code khó debug, tốn performance, mà còn làm cho mọi thứ khó kiểm soát và tam sao thất bản.

Đừng gọi magic function

Mức độ phổ biến: Rất hay gặp
Mức độ nghiêm trọng: Nó nhìn code của bạn rất chuối.
Như một ví dụ bên dưới, nó phản ánh sự không am hiểu của bạn về PHP.
################## KHÔNG NÊN ##############
$url = new BaseUrl();
$this->baseURL = $url->__invoke();
################## NÊN ##############
$url = new BaseUrl();
$this->baseURL = $url()
Magic function là một trong những đặc điểm vô cùng mạnh của PHP, mó làm cho PHP rất khác biệt so với các nền tảng khác, ví dụ như các hàm
  • __set
  • __get
  • __call
  • __toString
  • __isset
Bạn nên tìm hiểu đúng nó để sử dụng, và đừng nên gọi nó một các trực tiếp, vì nhìn nó chuối lắm.

Đừng cố gắng biến A thành A’

Mức độ phổ biến: Rất hay gặp
Mức độ nghiêm trọng: Nó nhìn code của bạn rất rờm rà.
Hãy nhìn vào đoạn code bên dưới:
################## KHÔNG NÊN ##############
class Product {
    protected $price;
    protected $category;
    protected $stock_status;

    public function getPrice(){
        return $this->price;
    }

    public function getCategory(){
        return $this->category;
    }

    public function getStock_status(){
        return $this->stock_status;
    }
}

So sánh với:

################## NÊN ##############
class Product {
  protected $price = 0;
  protected $category;
  protected $stock_status;

  public function __call($method, $arguments) {
    $prop = lcfirst(substr($method, 3));
    if( !property_exists($this, $prop) ){
      throw new \Exception('Property '. $prop . ' does not exist');
    }
    return $this->$prop;
  }
}

Ket:

Có thể bạn đã biết:

Tận dụng các magic function sẽ giúp cho code của bạn gọn gàng hơn, càng đơn giản bạn sẽ càng ít bug. Ngoài ra với một function tập trung kiểu này cũng sẽ giúp bạn kiểm soát các mistake dễ dàng hơn.

Lỗi về sử dụng switch-case

Mức độ phổ biến: Thỉnh thoảng hay gặp
Mức độ nghiêm trọng: Điều này dẫn tới rât khó maintain và phát triển thêm tính năng
Hãy nhìn vào đoạn code bên dưới, code này, việc code thành từng case như thế này mà input dầu vào chỉ có$keyvà$paramthì phần code này không thiết kế tốt sẽ rất khó thay đổi.
################## KHÔNG NÊN ##############
namespace CorePackage;

use CorePackage\Adapder\AdapterA;
use CorePackage\Adapder\AdapterB;
use CorePackage\Adapder\AdapterC;

class Service{

    public static function getSomething($key, $param = NULL){
        $config   = new Config();
        $options  = $config->getOptions();
        if (!in_array($key, $options) ){
            throw new \Exception('Invalid key');
        }
        switch((int)$options[$key]){
            case 0:
                $adapter  = new AdapterA();
                $data     = $server->getData($key, $param);
                break;
            case 1:
                $adapter  = new AdapterB();
                $data     = $adapter->getData($key, $param);
                break;
            case 2: 
                $adapter  = new AdapterC();
                $data     = $adapter->getData($key, $param);
                break;

        }
        return $data;
    }
}
################## NÊN ##############
namespace CorePackage;

use CorePackage\ Adapter\Factory;
use CorePackage\Adapder\AdapterInterface;

class Service{

    public static function getSomething($key, $param = NULL){
        $config   = new Config();
        $adapter  = Factory::create($config)
        if ( !($adapter instanceof AdapterInterface) ){
            throw new \Exception('Can not create adapater!');
        }
        $data = $adapter->getData($key, $param);
        return $data;
    }
}

Đừng code mọi thứ vào một cái hàm

Mức độ phổ biến: Khắp mọi nơi, mọi chỗ
Mức độ nghiêm trọng: Đây thật sự là một tội ác chông lại cộng đồng dev
Hãy nhìn vào đoạn code bên dưới, nhìn chúng gần như là lặp đi lặp lại một chuyện rất vô nghĩa.
################## KHÔNG NÊN ##############
function getUserAction($id)
  $userModel        = \App\Model\User::findById($id);
  $user = new \stdClass();
  $user->id         = $userModel->getId();
  $user->email      = $userModel->getEmail();
  $user->firstname  = $userModel->getFirst_name();
  $user->lastname   = $userModel->getLast_name();
  $user->fullname   = $user->firstname . ' ' . $user->lastname;
  $user->userstatus = 0;
  $user->birthday   = $userModel->getDob();
  $user->gender     = $userModel->getGender();
  $result['user']   = $user;
}

So sánh với:

################## NÊN ##############
function getUserAction($id)
  $userModel        = \App\Model\User::findById($id);
  $result['user']   = $userModel->toArray();
  // Hoac:
  $result['user']   = $userModel->asObject();
}

Ket:

  1. Các hàm quá dài, và không đóng gói sẽ dẫn tới việc code trở nên ngày càng khó đọc, không thể testing. Ngoài ra khi bạn viết trở thành function trong một class nào đó, bạn sẽ biết làm thế nào để optimize và code nó tốt nhất.
  2. Luôn có cách để chia các function dài thành các function nhỏ hơn, hãy có niềm tin như vậy.

Viết tất cả vào function hiện có

Mức độ phổ biến: Vô cùng phổ biến
Mức độ nghiêm trọng: Tội ác không thể tha thứ.
Có những function khi tôi đọc lại từ người khác, hay là nhận fix bug, hay viết thêm tính năng. Ôi thần linh ơi, không thể tin được, có hàng trăm, hàng ngàn dòng… Nhìn thật sự rất nản, rất chán, và rất muốn bỏ cuộc.
Bạn đừng bao giờ góp phần làm trầm trọng thêm tội ác đó.
Hãy xem một ví dụ:
################## NÊN ##############
function getSomethingAction($prarams = [], $options = []) {
  /**
   |------------------------------------------------------------
   | 1. Buoc 1 lam ABC
   | 2. Buoc 2 lam DEF
   | 3. Buoc 3 lam GHK
   |------------------------------------------------------------
   */   $result1 = doStep1($prarams, $options);
   $result2 = doStep2($prarams, $options);
   $result3 = doStep3($result1, $result2);
   return $result3;
}
function doStep1($prarams = [], $options = []) {
  /**
   |------------------------------------------------------------
   | 1. Buoc 1 lam ABC
   | 2. Buoc 2 lam DEF
   | n. Buoc n lam GHK
   |------------------------------------------------------------
   */   $result1 = doChildStep1_1($prarams, $options);
   $result2 = doChildStep1_2($prarams, $options);
   $result  = doChildStep1_N($result1, $result2);
   return $result;
}
function doStep2($prarams = [], $options = []) {
  /**
   |------------------------------------------------------------
   | 1. Buoc 1 lam ABC
   | 2. Buoc 2 lam DEF
   |------------------------------------------------------------
   */   $result1 = doChildStep2_1($prarams, $options);
   $result  = doChildStep2_2($result);
   return $result;
}
// Va cac function khac