2
\$\begingroup\$

This is just my simple structure. I want to know if there is a better way than this because I have so many lines of code already.

    <?php
$p1speed = 5;
$p2speed = 3;

//way for next turn: 1 for turn
$p1 = 0;
$p2 = 0;

if($p1speed>$p2speed) {

    $p1 = 1;
    $p2 = 0;

}
else if($p2speed>$p1speed) {

    $p1 = 0;
    $p2 = 1;

}

$count=0;

//begin
while($count<5){

    if($p1==1) {

        echo 'P1 turns <br>';

        //adjust
        $p1 = 0;
        $p2 = 1;

    }
    else if($p2==1) {

        echo 'P2 turns <br>';

        //adjust
        $p1 = 1;
        $p2 = 0;

    }

    $count++;

}
?>
\$\endgroup\$
3
  • \$\begingroup\$ What are you trying to achieve? Is the 5 in the while loop fixed? \$\endgroup\$
    – miniBill
    Sep 12 '12 at 7:09
  • \$\begingroup\$ yes, A battle game \$\endgroup\$ Sep 12 '12 at 7:13
  • \$\begingroup\$ Why are you giving the choice of PHP, C, and C++? The languages are utterly different. \$\endgroup\$ Sep 12 '12 at 12:38
2
\$\begingroup\$

Why have to tagged this as C and C++? There is nothing here to do with either language, this is all PHP.

Why are you explicitly declaring an elseif statement when comparing speed? The only reason I can think of is if you are also going to compare for equality (going same speed). But you aren't, so an else statement would serve a better purpose here by comparing for both equality and less-than. Unless the initial "turn" variable values of 0 are supposed to cover this eventuality, but then they should be defined in a final else, otherwise this just looks odd. Besides, the while loop makes me think this is not the case.

if( $p1speed > $p2speed ) {
    //change turns
} else {
    //equivalent to if( $p2speed >= $p1speed ) 
}
//OR
if( $p1speed > $p2speed ) {
    //change turns
} elseif( $p2speed > $p1speed ) {
    //change turns
} else {
    //default turns
}

Extending Player Pool

Some of the following might be beyond the scope of your project, but figured I'd mention it anyways, as its something to think about when developing a game, or any kind of application. Does my code allow for expansion? Is expansion necessary? Richard has already pointed out a better way to treat turns (+1), but the problem with his implementation is that it does not easily allow for more than two players. To accomplish this, you will need a counter, which you could reuse from the loop, and some sort of operation to reset the index when the counter has exceeded maximum number of players, I'm thinking modulo. So for example, using Richard's example we can easily determine who's turn it is on every iteration by using the following bit of code inside the loop.

$currentPlayer = $counter % $numberOfPlayers;

How then do we determine which player is first with a larger player pool? Which is second? We can't manually write each comparison. Well, we could, but that would be a bit much. Instead, if we get all the player speeds into an array, we can sort it and use the array indices to determine the order. Of course, that means we will have to tweak the above algorithm to use the array positions instead of a sequential incremental to ensure players going faster are not behind players going slower, but that's simple enough.

$speeds = array(
    5,//player 1
    3,//player 2
    2//player 3
);
$numberOfPlayers = count( $speeds );
asort( $speeds );
//do other things and eventually start loop
$position = $counter % $numberOfPlayers;
$currentPlayer = $speeds[ $position ];

The problem with the above example is that the players are already in order, so it does not demonstrate this very well. But take my word for it, those numbers could easily be mixed up and still work. asort() is the key here. It sorts an array and keeps the array's indices in tact.

Loops

While loops are for testing a condition until otherwise (dis)proven. In other words, "is this statement TRUE?" Loop until it is. Typically this is done for things that already have this kind of boolean type associated with it and just need certain conditions to be met before it is passed. Such as when reading from a file. The problem with these kinds of loops is that it is very easy to accidentally create an infinite loop and crash the program. The need for a while loop should be rare.

For loops are for looping over a set of sequential data using an explicit counter.

Foreach loops are for looping over a set of non-sequential data using an internal (invisible) counter.

So, the question here is: Why are you recreating a for loop with a while loop? As miniBill pointed out, a for loop is better here. Hopefully the above explanations can help you figure out why. If your concern is speed, know that PHP has come a long way since the early days and for loops are now much better.

for( $i = 0; $i < 5; $i++ ) {

Cleaning Up

The main problem you seem to be having right now is that you are using procedural code, which, as you have noticed, results in a lot of bulky, repetitive code. The only cure for this is to start using a class or functions. I'd lean more towards a class here, but if you are unfamiliar with the concept, learning how to implement functions properly might be a better first step. Just for the sake of example, here is a turn toggle as I would write it in a class method. Of course, there is a better way to do this, as we have already discussed, but it serves to demonstrate the point quite well.

public function toggleTurns() {
    $this->p1 = $this->p1 == 1 ? 2 : 1;
    $this->p2 = $this->p2 == 1 ? 2 : 1;
}

//in some other method
for( $i = 0; $i < 5; $i++ ) {
    if($p1==1) {
        echo 'P1 turns <br>';
    } else {
        echo 'P2 turns <br>';
    }

    $this->toggleTurns();
}
\$\endgroup\$
2
\$\begingroup\$

This is a refactoring of the original code. It picks the starting player and then uses a simple toggle ($currentPlayer = ($currentPlayer == 1) ? 2 : 1;) to determine which player goes next for 5 rounds.

$currentPlayer = 1;

if ($player2speed > $player1speed) {

    $currentPlayer = 2;

}

$count = 0;

while ($count < 5) {

    echo $currentPlayer . " turns<br/>";

    $currentPlayer = ($currentPlayer == 1) ? 2 : 1;

    $count ++;
}

Note, the current code fails if both player's speeds are the same.

\$\endgroup\$
3
  • \$\begingroup\$ Would you please add more detail to your answer as to how you would improve the code? As it stands this answer is mostly just a code dump... \$\endgroup\$
    – Phrancis
    Mar 23 '15 at 14:41
  • \$\begingroup\$ @Phrancis It's just a re-factoring of the original code mainly to redress the 'I want to know if there is a better way than this because I have so much of line code already please help' It picks the starting player and then uses a simple toggle ($currentPlayer = ($currentPlayer == 1) ? 2 : 1;) to determine which player goes next for 5 rounds. \$\endgroup\$ Mar 23 '15 at 15:39
  • 1
    \$\begingroup\$ Thanks for clarifying. Also just noticed you posted this 3 years ago, nice to see you're still around! \$\endgroup\$
    – Phrancis
    Mar 23 '15 at 16:13
0
\$\begingroup\$

From: Intel 64 and IA-32 Architectures Optimization Reference Manual

Intel 64 and IA-32 Architectures Optimization Reference Manual
Chapter 3 GENERAL OPTIMIZATION GUIDELINES
3.4.1 Branch Prediction Optimization

Eliminate branches whenever possible.

$toggle = array(1,0);
$turns = array(0=> array(0=>'P1 Turns<br/>',1=>''),
               1=> array(0=>'P2 Turns<br/>',1=>''));



for( $i = 0; $i < 5; $i++ ) {
  $p1 = toggle[$p1];
  $p2 = toggle[$p2];
  echo $turns[$p1][$p2];
}
\$\endgroup\$
-1
\$\begingroup\$

If the 5 in the while loop is fixed you can simply unroll it as: [C code, but I hope you get the idea]

void turn(int player);

void foo(){
    int p1speed = 3;
    int p2speed = 5;

    int first_player;
    int second_player

    if(p1speed > p2speed){
        first_player = 1;
        second player = 2;
    }
    else{
        first_player = 2;
        second_player = 1;
    }

    //now, instead of the while loop you can have:
    turn(first_player);
    turn(second_player);
    turn(first_player);
    turn(second_player);
    turn(first_player);
}

void turn(int player){
    printf("Player %d turn", player);
    int other = 3 - player; // Nice trick
    //Do something
}
\$\endgroup\$
3
  • \$\begingroup\$ fix but not 5 , 30 instead :) thanks! \$\endgroup\$ Sep 12 '12 at 7:43
  • \$\begingroup\$ well, unrolling a 30 loop makes less sense, but you could still use a for(int i = 0; i < 15; i++){turn(first_player);turn(second_player);} \$\endgroup\$
    – miniBill
    Sep 12 '12 at 8:06
  • \$\begingroup\$ This does not sound like a very portable alternative... Can you please expand on why you would do it this way? \$\endgroup\$
    – Phrancis
    Mar 23 '15 at 14:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.