2010-12-21 11 views
2

私はこのsms通知システムを構築しています。これは、特定の機会に基づいて10回の無料SMSをWebメンバーに送信し、特定のメンバーが10回到達した後、システムは、 「これが最後の無料のSMS通知である」、私は現在、PHP OOPを学習し、さらにここで行うことなく、このPHP OOPはアドバイスが必要です

にOOPのaproachを使用しようとしていますと、私のコードです:

<?php 
class SmsBonus { 
//bonus_sms fields = id, member_id, counter, end_status 

public static function find_member($id=0){ 
    //query to find a certain member 
} 

public function add_counter($id=0){ 
    //query to increment the value of counter field 
} 

public function status_check($id=0){ 
    //query to check whether the given member's counter has reach the number 10 
} 

public static function send_sms($id, $message){ 
    $found = $this->find_member($id); 
    $status_check = $this->status_check($id); 

    if(!empty($found) && !empty($status_check) && $found->counter == 10){ 
     //send the sms notification saying that this member has reach the end of the bonus period 

     //update this member's end_status table to 1 
    }else{ 
     //send the regular notification 
    } 
} 

} 
?> 

だろう、この行:

$found = $this->find_member($id); 
$status_check = $this->status_check($id); 

は期待どおりに動作します(私は現時点ではローカルでこれを構築しているため、これをテストできません)。これはOOPに関するベストプラクティスですか?または私はこれを間違っている?

アドバイスが必要です、どうもありがとうございます。

EDIT:私の元のコードのコースの

私はクラスを宣言し、私はここでそれを書いていないことにより、誰もが混乱することを申し訳ありません:Dを、私は実際に指摘答えのようなもの(アドバイス)を探しています私のコード(この場合はメソッド)で最良のアプローチ(ベストプラクティス)を実装する必要があります。心配するのは、私がKISSやDRYなどの要件を満たしていないことです

更新 私はあなたの提案に基づいていくつかの変更を行うことができますが、どのように見えますか?

<?php 
    class SmsBonus{ 
     //bonus_sms fields = id, member_id, counter, end_status 
     protected $max_sms = 10; 

     public $id; 
     public $member_id; 
     public $counter; 
     public $end_status; 

     public function find_member($id=0){ 
      //query to find a certain member 
     } 

     public function add_counter($id=0){ 
      //query to increment the value of counter field 
     } 

     public function status_check($id=0){ 
      //query to check whether the given member's counter has reach the number 10 
     } 


     public function update_status($id=0){ 
      //query to update when a certain member reach its sms bonus limit 
     } 

     protected function can_still_send_sms($member_id){ 
      $found   = $this->find_member($member_id); 
      $status_check = $this->status_check($id); 
      return !empty($found) && $found->counter < $this->max_sms && !empty($status_check); 
     } 

     public function send_sms($id, $message){ 
      $phone = Phone::find_member($id); // 
      if ($this->can_still_send_sms($id)) {  
       //send the sms notification saying that this member has reach the end of the bonus period 

       $this->update_status($id); 
      }else{    
       //send the regular notification 

       $this->add_counter($id); 
      } 
     } 
    } 
    $sms_bonus = new SmsBonus(); 
?> 

答えて

2

OOPは主に、再利用が簡単な意味のあるアクションを作成することに重点を置いており、特に数か月後にコードを再訪したときに何が起きているのかを簡単に見つけ出すことができると思います。それは多かれ少なかれ同じです)。また、memberが見つかったら、idではなく、そのロジックを実行できます。したがって、この場合には、たとえば、このようなあなたのメソッドを作成する方がよいかもしれません:

protected $max_sms_messages = 10; 

protected function can_still_send_sms($member){ 
    return !empty($member) && $member->counter < $this->max_sms_messages; 
} 

public function send_sms($id, $message){ 
    $found = $this->find_member($id); 
    if ($this->can_still_send_sms($found)) { // or even if($found->can_still_send_sms()), if you want to implement it that way 

     //send the sms notification saying that this member has reach the end of the bonus period 

     //update this member's end_status table to 1 
    }else{ 
     //send the regular notification 
    } 
} 

また、記録のために、あなたは、静的メソッドから非静的メソッドを呼び出すことはできません。

+0

私は質問をどのように更新する必要がありますようにクラスを宣言するときに2つの角かっこを置くしたくないそれ ?ありがとう – littlechad

1

あなたは、クラス宣言

class SMSNotification { 
... 
} 

でコードをラップする必要があります。また、ユーザーが設定できるように、このため

function __construct() { 

一つの理由があるのコンストラクタを作成することもできますインスタンス化されたときにプライベート変数をクラスに追加します。

あなたは次のようにクラスをインスタンス化:

$sms = SMSNotification() 

あなたはカウンターの増分のためのデータベースにこれを接続する予定。 oopのアプローチで普通にやっているのは、その接続を扱う別のクラスを持っているからです。このプロジェクト全体を構築したいのであれば、すべて同じ方法でデータベースに接続することになります。あなたが貼り付けられたコードの

2行は少し異なります。

$found = $this->find_member($id); 

あなたが作成せずに関数を呼び出すことができるようにあなたが(私が行っているだろうか、おそらくです)静的関数find_memberてきました新しいクラスオブジェクト。それは現在のインスタンス化されたクラスの一部ではないので、$ thisの価値がないと言われています。だから、(SMSNotificationの私の例を使用して)のようにそれを呼び出す必要があります:

$found = SMSNotification::find_member($id); 

これは、コードの他のラインが正常に動作する必要があり

find_memberという静的機能を探すためにPHPを告げる:

$status_check = $this->status_check($id); 
+0

私はクラスを宣言しました、私はちょうど私の質問にそれを書いていない、またインスタンス化する方法も知っていますが、指摘のおかげで:D – littlechad

+0

大丈夫です。あなたはあなたが上記の "クラスSmsBonus(){"は "SmsBonus {" – dewy

+0

オハイオ州、私は急いでいた誤解をする必要があります:p – littlechad

0

OOPによれば、静的メンバー関数で$this->find_member($id)を呼び出すことはできません。あなたはクラスを宣言していないだけでなく、$thisは無意味です(私がPHPを覚えている限り)。あなたは、メンバー変数でのDBクエリの充填から初期化されるいくつかのSmsClientクラスを宣言したいと思っていました。あなたの静的find_member($id=0)機能がデウィを聴く$id

class SmsClient 
{ 
private $id; 
private $nSmsSent; 

public __construct($id) 
{ 
    $res = DAL->GetClient($id); 
    //initialize vars here 
} 

public send_sms(...) 
{ 
    $this->nSmsSent++; 
} 
} 
0

= idでDBを照会し、そのIDをSmsClientの初期化されたオブジェクトを返します。

正しい構文を使用しているかどうかをテストするには、find_member()status_check()関数の内容をコメントにして、いくつかの値を返すようにしてください。実際に値が返されると、右。

関連する問題