2011-08-02 54 views
0

这里是代码...我的if,else语句很长...如何区分它?任何建议?谢谢。如何使此代码更好地组织

public function receiveMsg(aMsg){ 
    if($aMsg instanceof LoginMsg){ 
     $this->callingSomeMethod();   //should I separate this code into other class/ object? 
     $this->callingAnotherMethod(); //should I separate this code into other class/ object? 

     $aMsg = new RespondLoginMsg(); //should I separate this code into other class/ object? 
     $this->sendMsg($aMsg);   //should I separate this code into other class/ object? 

    }else if(aMsg instanceof LogoutMsg){ 
     $this->callingSomeMethod();  //should I separate this code into another class/ object? 

     $aMsg = new RespondLogoutMsg();  //should I separate this code into another class/ object? 
     $this->sendMsg($aMsg);    //should I separate this code into another class/ object? 


    }else if/*****bababab***/ 

    /*****many else if here (up to 200 msg+ , just upload 2 here.)***/ 


} 

回答

0

我没有看到任何根本错误的长if/else语句,如果这就是所谓的。

如果你发现它是一个眼睛,为什么不直接定义每个if/else块的内容作为它自己的函数呢?

你也可以考虑消除重做。如果它们全都以

 $aMsg = new RespondLoginMsg(); 
    $this->sendMsg($aMsg); 

那么就不需要在每个块中重复它。

+0

不,你可以看到,第二个是RespondLogoutMsg而不是RespondLoginMsg ..... – Tattat

+0

啊哈。错过了,对不起。 – craigmc

2

也许一个开关更容易阅读?而SENDMSG可能被移出它无论哪种方式,只是使用您的马桶盖设置$ AMSG对象/如果..

$strMessageClass=get_class($aMsg); 
switch ($strMessageClass) { 
    case 'LoginMsg': 
     $this->callingSomeMethod(); 
     $aMsg = new RespondLoginMsg(); 
    case 'RespondLogoutMsg': 
     $this->callingAnotherMethod(); 
     $aMsg = RespondLogoutMsg(); 
    default: 
     // If you have any.. 
} 
$this->sendMsg($aMsg); 
0

除了摆脱过度间距和换行的我没有看到这里的问题是什么。你需要检查一个项目是否是一个实例的多少个不同的类?在我看来,这里的名单会相当有限。如果您没有超过3个或4个if/else语句,那么只需将其保留为if/else即可。否则使用开关或循环。

你可以更具体地说明你正在努力完成什么?

这是一个稍微更清晰的代码版本。

public function receiveMsg(aMsg) { 
    if ($aMsg instanceof LoginMsg) { 
    $this->callingSomeMethod(); 
    $this->callingAnotherMethod(); 

    $aMsg = new RespondLoginMsg(); 
    $this->sendMsg($aMsg); 
    } 
    else if (aMsg instanceof LogoutMsg) { 
    $this->callingSomeMethod(); 

    $aMsg = new RespondLogoutMsg(); 
    $this->sendMsg($aMsg); 
    } 
    else if { /*****bababab***/ 

    } 
    /*****many else if here***/ 
} 
+0

我减少了源代码,我有高达200 msg ....他们工作方式类似,所以,我只是复制一些,并在这里发布。 – Tattat

+1

你有多达200种不同类型的信息?因为我在谈论类型。这意味着你有200个不同的课程。那太过分了。您可能只需创建一个消息类并为该类中的每个消息类型创建一个函数。然后,您可以创建一个循环来检查类型并调用相应的函数。这个功能将会使所有需要的子调用。这样你就不必把它放在这个函数中。保持代码更清洁,更可重用。 – pthurmond