2016-09-23 22 views
2

私はWeb開発者であり、PHP/Laravelフレームワークでコードを記述しています。私はコードを作成するためのベストプラクティスに従うことを目指してきました。クラス内に15〜20行のコードと最大200行のコードを書くのは良い方法です。しかし、毎回、関数に最低40〜50行を書くことになります。ここでは、クライアントと割り当てられたユーザーの詳細を取得するために書いたコードスニペットを示します。PHPでコード行を減らすにはどうしたらいいですか?

public function preMessageSend($client, $assigned) 
{ 
    $ticket_number = $client->ticket_number; 
    $title = $client->title; 
    $department = $client->department; 
    $priority = $client->priority; 
    if ($client->first_name !== null || $client->first_name !== '') { 
     $client_name = $client->first_name." ".$client->last_name; 
    } else { 
     $client_name = $client->username; 
    } 
    if ($client->email !== '' || $client->email !== null) { 
     $client_email = $client->email; 
    } else { 
     $client->email = 'Not available'; 
    } 
    if($client->mobile !== null || $client->mobile !== '') { 
     $client_mobile = $client->code."".$client->mobile; 
    } else { 
     $client_mobile = 'Not available'; 
    } 
    if($assigned != null) { 
     if ($assigned->first_name !== null || $assigned->first_name !== '') { 
      $assigned_name = $assigned->first_name." ".$assigned->last_name; 
     } else { 
      $assigned_name = $assigned->username; 
     } 
     if ($assigned->email !== '' || $assigned->email !== null) { 
      $assigned_email = $assigned->email; 
     } else { 
      $assigned->email = 'Not available'; 
     } 
     if($assigned->mobile !== null || $assigned->mobile !== '') { 
      $assigned_mobile = $assigned->code."".$assigned->mobile; 
     } else { 
      $assigned_mobile = 'Not available'; 
     } 
     if ($assigned->address !== null || $assigned->address !== '') { 
      $assigned_address = $assigned->address; 
     } else { 
      $assigned_address = 'Not available'; 
     } 
     $this->sendMessageWithAssigned($ticket_number, $title, $department, $priority, $client_name, $client_email, $client_mobile, $assigned_name, $assigned_email, $assigned_mobile, $assigned_address); 
    } else { 
     $this->sendMessageWithoutAssigned($ticket_number, $title, $department, $priority, $client_name, $client_email, $client_mobile); 
    } 

私は私のクラスや関数でLOCを削減し、このような長い関数を書く回避するためのベストプラクティス何ができる方法を教えてください。 TIA

あなたが行うことができ
+1

最初の質問:なぜこれらのクライアントオブジェクトのプロパティをすべてローカルスコープの変数に割り当てる必要がありますか? –

+0

私はあなたが 'nullではなく空の文字列ではなく、空のチェックをすることができると思います。 – danopz

+0

ほとんどの条件を三項演算子に書き直すことができます。 $ client-> mobile!== '')? $ client-> code: "$ client-> mobile: '利用できません'; ' –

答えて

0

代わりの

if ($client->first_name !== null || $client->first_name !== '') { 
    $client_name = $client->first_name." ".$client->last_name; 
} else { 
    $client_name = $client->username; 
} 

$client_name = ($client->first_name !== null || $client->first_name !== '') ? $client->first_name." ".$client->last_name : $client->username; 
0

まず、null''はそうあなたができるempty()ためtrueです:

if (!empty($client->first_name)) { // if not empty 
    $client_name = $client->first_name." ".$client->last_name; 
} else { 
    $client_name = $client->username; 
} 

その後あなたはまた、彼は三項演算子:

$client_email  = $client->email or 'Not available'; 
$client_mobile = $client->code . $client->mobile or 'Not available'; 
$assigned_address = $assigned->address or 'Not available'; 

これらor文ですのみ等しい:次に

$client_name = !empty($client->first_name) ? $client->first_name." ".$client->last_name : $client->username; 

一部文ために利用可能なorの文もあり

if(!empty($assigned->address)){ 
    $assigned_address = $assigned->address; 
} else { 
    $assigned_address = 'Not available'; 
} 

// Or the equivalent ternary 
$assigned_address = !empty($assigned->address) ? $assigned->address : 'Not available'; 

そして、私が「何人か」を意味するのはそれです:

$client->first_name = null; 
$client->last_name = null; 
echo empty($client->first_name." ".$client->last_name); // false 
echo isset($client->first_name." ".$client->last_name); // true 

が原因!empty()は常にisset()場所として反対の結果が得られていないので、それがisset()

今すぐそれらまたは文に注意してくださいになるだろう" "空間に、両方の変数がnullの場合でも、空ではありませんisset([])が真で、empty([])も真です。

0

すでに提案されているように、!= null!= ''の代わりにempty()を使用できます。さらに、あなたは例えば、ほとんどの文でelse一部を省略することができ:

$assigned_name = $assigned->username; 
if (!empty($assigned->first_name)) { 
    $assigned_name = $assigned->first_name." ".$assigned->last_name; 
} 

これは、デフォルトでは、あなたの元else値に$assigned_nameを設定し、条件が満たされた場合$assigned_nameが上書きされます。私はそれが読めるIMOではないので、三項演算子の使用をお勧めしません。

コードが読みやすく効率的であれば、コードの行についてはあまり心配する必要はありません。

関連する問題