Refactoring of this complex PHP method

Question

I have one complex method with 1890 paths. I don't know how can be this refactored. Can somebody add some tips about it? Maybe the MySQL table structure is wrong.

public function fetchClubChallengeState()
{
 $c = $this->caller->in_club;
 $o = $this->opponent->in_club;
 if (!$c || !$o) {
 return false;
 }
 $last = Yii::app()->db->createCommand()
 ->select('*')
 ->from('challenge')
 ->where('(caller=:caller OR caller=:opponent) AND winner=0', [':caller'=>$c, ':opponent'=>$o])
 ->limit(1)
 ->queryRow();
 if ($last['caller']==$c and $last['opponent']==$o or $last['caller']==$o and $last['opponent']==$c) {
 $created = strtotime($last['created']);
 if ($this->isBetweenDates($created + 1800, $created + 3600)) {
 $this->challengeID = (int)$last['id'];
 $this->callersClubRole = $last['caller'] == $c ? 'caller' : 'opponent';
 $this->callersClub = $last['caller'] == $c ? $last['name_caller'] : $last['name_opponent'];
 $this->opponentsClubRole = $last['opponent'] == $o ? 'opponent' : 'caller';
 $this->opponentsClub = $last['opponent'] == $o ? $last['name_opponent'] : $last['name_caller'];
 }
 }
}

Edit:
The structure of the challenge table:

CREATE TABLE `challenge` (
 `id` int(11) NOT NULL AUTO_INCREMENT,
 `caller` mediumint(7) unsigned NOT NULL DEFAULT '0',
 `opponent` mediumint(7) unsigned NOT NULL DEFAULT '0',
 `name_caller` varchar(20) NOT NULL DEFAULT '',
 `name_opponent` varchar(20) NOT NULL DEFAULT '',
 `winner` tinyint(1) unsigned NOT NULL DEFAULT '0',
 `created` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
 //...
PRIMARY KEY (`id`),
KEY `c_o` (`caller`,`opponent`),
KEY `winner` (`winner`)
) ENGINE=InnoDB AUTO_INCREMENT=11 DEFAULT CHARSET=utf8;

If the challenge was started by Player1's club, that clubs ID goes to the caller field and the ID of the opponents club goes to the opponent field. These two IDs are checked here:
if ($last['caller']==$c and $last['opponent']==$o or $last['caller']==$o and $last['opponent']==$c) { This means, that the found challenge is between the clubs of the 2 players.

The code is also here.

• nrob: I'we extracted the bottom part to other method, but I think, it can be refactored further.
• Phrancis: It's possible that MySQL is the culprit, but we cannot tell until you post some code that calls MySQL.
• 200_success: A SELECT that has a LIMIT but no ORDER BY is rarely useful. Are you sure that your query produces the right result, deterministically?
• nrob: I am sure, that the query returns the right data, because it can be only one active challenge (winner=0) of a club.

Answer 1

Don't know what happens in your db, but LIMIT 1 suggests that result of your query will return exactly what you ask or nothing. In this case no need to compare caller and opponent in if statement - just return (false) from method when $last is empty/false.

Otherwise if one caller may have more 'winner = 0' opponents LIMIT 1 could give you result that you won't use (if statement will reject it) while there's a valid one but was cut off by limit. In this case better provide full description in where clause (if statement will also check if there is any result).

• nrob: I had edited my post and added the scheme of the table.
• shudder: It's not about structure itself, but there's definitely some logic issue here. My point is: You're selecting one particular record from db and yet still performing checks if this record is that what you wanted. Why don't you write your query more precise and skip all the checks? I don't mean ternary caller/opponent assign here since both cases are equally handled. Btw. I understand that $c = $this->caller->inClub; and $o come from assumption and they don't neccessary match roles in challenge, right? Is it checking known club against selected one? Naming is fuzzy here.
• nrob: Hmm, you are right. I need to write a more precise query to eliminate the checks in the PHP code. I don't need to find a row, where the opponent is other, than $this->opponent->inClub. Thank you!