2014年6月6日 星期五

Opencart – 又臭又長的程式碼

使用後台 controller 的 order.php 裡面的錯誤處理來當例子。

檔案: /admin/controller/sale/order.php 表單在送出之後,會用到 update(),這裡面會檢查表單是否送出。

        if (($this->request->server['REQUEST_METHOD'] == 'POST') && $this->validateForm()) {
            $this->model_sale_order->editOrder($this->request->get['order_id'], $this->request->post);

            $this->session->data['success'] = $this->language->get('text_success');

            //以下略
        }

if 裡面,會用到validateForm(),裡面會對使用者輸入的欄位值做檢查。取一小段如下

        if ((utf8_strlen($this->request->post['firstname']) < 1) || (utf8_strlen($this->request->post['firstname']) > 32)) {
            $this->error['firstname'] = $this->language->get('error_firstname');
        }

它會把所有的錯誤,寫到  $this->error 陣列。
然後在另一個函數 getForm(),也就是 update() 的下一步,在末端結尾前有一大段在做錯誤處理

        if (isset($this->error['firstname'])) {
            $this->data['error_firstname'] = $this->error['firstname'];
        } else {
            $this->data['error_firstname'] = '';
        }

這僅僅是其中一個欄位而已。所有要檢查的欄位加起來,很多很多行。我覺得這種處理方式很不好。等於一個欄位處理兩次,在validateForm() 裡面處理 1 次,到了 getForm() 又處理 1 次。這可能是因為:

validateForm() 設定的是 $this->error 陣列,getForm() 設定的是 $this->data 陣列。而 $this->data 是模版頁面所需要的陣列,用來輸出所有資料。所以不能直接讓 $this->data = $this->error,因為 $this->data 包括整個頁面由 php 產生的資料。

在 $this->data 所設定的資料,到了模版頁面要用鍵值當成變數來取用。例如 $this->data['firstname'] ,到了 order_info.tpl,要使用 $firstname 。

大概是因為這樣,所以每一個錯誤訊息都用了一小段程式碼來處理。不止錯誤訊息是這樣,過濾條件也是 這樣(filter_order_id, filter_order_status_id ……),轉址($url)也是這樣。使得程式碼最後變的又臭又長。這樣的缺點是, 在改寫的時候要在這些又臭又長的程式碼之間不斷的滾、滾、滾,不斷的滾動滑鼠。

我覺得應該善用陣列。例如客戶資料,應該使用 $this->data['customer'],這樣會有 $this->data['customer']['firstname'], $this->data['customer']['lastname']……等。雖然變數名稱比較長,但是非常好維護。在模版頁,要除錯的時候,只要用這一行

echo "<pre>", print_r($customer, 1), "</pre>"; exit;

就可以把所有資料都輸出。(這一行是我非常愛用的萬能除錯工具。)

錯誤訊息則是這樣:

 $this->data['error'] = $this->error;

像這樣 一行就解決了,多麼輕鬆愉快!

1 則留言: