Ticket #964 (closed 新規開発: 修正済)

Opened 10 years ago

Last modified 10 years ago

[管理画面]基本情報管理

Reported by: kotani Owned by: coelacanth
Priority: Milestone: EC-CUBE2.11.0 リファクタリングProject
Component: 管理画面 Version: 2.11.0 β
Keywords: Cc:
修正済み:

Description (last modified by kotani) (diff)

▼リファクタリング対象ファイル
/admin/basis/LC_Page_Admin_Basis.php
/admin/basis/LC_Page_Admin_Basis_Control.php
/admin/basis/LC_Page_Admin_Basis_Delivery.php
/admin/basis/LC_Page_Admin_Basis_Delivery_Input.php
/admin/basis/LC_Page_Admin_Basis_Holiday.php
/admin/basis/LC_Page_Admin_Basis_Kiyaku.php
/admin/basis/LC_Page_Admin_Basis_Mail.php
/admin/basis/LC_Page_Admin_Basis_Payment.php
/admin/basis/LC_Page_Admin_Basis_Payment_Input.php
/admin/basis/LC_Page_Admin_Basis_Point.php
/admin/basis/LC_Page_Admin_Basis_Seo.php
/admin/basis/LC_Page_Admin_Basis_Tradelaw.php
/admin/basis/LC_Page_Admin_Basis_ZipInstall.php


リファクタリングガイドライン
http://svn.ec-cube.net/open_trac/wiki/リファクタリングガイドライン


Change History

comment:1 Changed 10 years ago by kotani

  • Description modified (diff)

comment:2 Changed 10 years ago by coelacanth

  • Owner changed from somebody to coelacanth
  • Status changed from new to assigned

comment:3 Changed 10 years ago by kotani

coelacanth様

株式会社ロックオン小谷です。お世話になっております。
リファクタリング開発にご協力頂きありがとうございます!

早速ではございますが、r20168 でコミット頂きましたソースを確認させて頂きました。
1点ご確認頂きたい部分がございましたので、コメントさせて頂きます。

/data/class/pages/admin/basis/LC_Page_Admin_Basis_Holiday.php
112行目:$arrHolidayData = $this->lfGetHolidayDataByHolidayID($_POSTholiday_id?);
など、
$_POST の値が入力チェックせずに使用されている部分が何か所かあるようです。
大変恐縮ですが、ご修正頂けますとありがたいです。

また、
/data/class/pages/admin/basis/LC_Page_Admin_Basis_ZipInstall.php
145行目:function lfAutoCommit() {
のように、関数名が Prefix + 動詞 + 対象 になっておりません。
http://svn.ec-cube.net/open_trac/wiki/EC-CUBE標準規約#関数名 をご参考頂ければ幸いです。

お忙しい中ご協力頂いており大変恐縮ではございますが、ご確認の程よろしくお願い致します。

comment:4 Changed 10 years ago by coelacanth

株式会社ロックオン小谷様

お世話になっております。 入力値のバリデートについて了解しました。

関数名が Prefix + 動詞 + 対象 になっておりません。

規約を確認の上修正いたいします。

よろしくお願いします。

comment:5 Changed 10 years ago by coelacanth

  • Status changed from assigned to closed
  • Resolution set to 修正済

comment:6 Changed 10 years ago by kajiwara

  • Status changed from closed to reopened
  • Resolution 修正済 deleted

株式会社ロックオンの梶原です。

リファクタリング作業、本当に御苦労さまです。
(かなり、ファイル数があったかと思いますが、対応していただき、誠にありがとうございます。)

ただ、申し訳ございませんが、リファクタリングガイドラインに沿ってない部分がございまして、差し戻しさせていただきます。
細かな部分は追ってご連絡させていただきますが、まずは概ね以下のチケットと同様の部分の指摘になると思いますので、取り急ぎご確認いただければと思います。

#979

リファクタリング作業は2/23(水)中までになりますので、何卒よろしくお願い致します。

それでは、どうぞよろしくお願い致します。

comment:7 Changed 10 years ago by coelacanth

株式会社ロックオン梶原さま

お世話になっております、西村です。

ただ、申し訳ございませんが、リファクタリングガイドラインに沿ってない部分がございまして、差し戻しさせていただきます。

こちらも把握不足なところがありますので、どんどん差し戻して指摘ください。もしくはこちらでリファクタリングの終了を判断せずにチェック後に完了というのが良さそうですね。

#979を確認させていただきましたが、

(1)http://svn.ec-cube.net/open_trac/wiki/リファクタリングガイドライン#端種別の判別

  • Net_UserAgent_Mobile::isMobile() や MOBILE_SITE 定数など存在するが, SC_Display::detectDevice() に統一する

(2)http://svn.ec-cube.net/open_trac/wiki/リファクタリングガイドライン#データベースアクセス

  • 特に理由の無い場合はSC_Query::getSingletonInstance() を使用してインスタンスを取得すること

(3)http://svn.ec-cube.net/open_trac/wiki/リファクタリングガイドライン#ビジネスロジック

  • 宣言を除き, 引数や返り値が無く, すべて内部のメンバ変数で処理するような関数は極力作成しない

の(3)が主なガイドラインに沿ってない部分ということでよろしいでしょうか?

細かな部分は追ってご連絡させていただきますが、

お忙しいとは思いますが、よろしくお願いします。

comment:8 Changed 10 years ago by coelacanth

Todo;

関数内で直接POSTを利用している処理の除去 関数内でメンバー変数を直接使用している箇所を引数として渡すように変更 関数内でreturnせずにメンバー変数に代入しているような処理の修正

comment:9 Changed 10 years ago by coelacanth

#979のコメントを参考にリファクタリングしました

comment:10 Changed 10 years ago by kotani

  • Status changed from reopened to closed
  • Resolution set to 修正済
Note: See TracTickets for help on using tickets.