Coding best practice?

[celfred]

Hello,

My code seems to become quite messy very quickly when I do something...

For example :

    $taskId = $input->post->task[$checked_player];
    if ( $taskId != 0) { // Task selected
      // Get the task bonuses
      $task = $pages->get($taskId)->title;
      $new_XP = $pages->get($taskId)->XP;
      $new_HP = $pages->get($taskId)->HP;
      $new_GC = $pages->get($taskId)->GC;
      // Do the math
      $player->XP = $player->XP + $new_XP;
      $player->HP = $player->HP + $new_HP;
      $player->GC = $player->GC + $new_GC;
    }

I feel like I'm using to many 'bridges' variables (I'm not sure this is clear)...

Do you think the following would be better? :

    if ( $input->post->task[$checked_player] != 0) { // Task selected
      // Get the task bonuses
      $task = $pages->get($input->post->task[$checked_player])->title;
      $new_XP = $pages->get($input->post->task[$checked_player])->XP;
      $new_HP = $pages->get($input->post->task[$checked_player])->HP;
      $new_GC = $pages->get($input->post->task[$checked_player])->GC;
      // Do the math
      $player->XP = $player->XP + $new_XP;
      $player->HP = $player->HP + $new_HP;
      $player->GC = $player->GC + $new_GC;
    }

Or even (not tested) :

   if ( $input->post->task[$checked_player] != 0) { // Task selected
      $task = $pages->get($input->post->task[$checked_player])->title;
      // Do the math
      $player->XP = $player->XP + $pages->get($input->post->task[$checked_player])->XP;
      $player->HP = $player->HP + $pages->get($input->post->task[$checked_player])->HP;
      $player->GC = $player->GC + $pages->get($input->post->task[$checked_player])->GC;
    }

Is it just personal preferences or does it have a performance issue to use variables ?

It may sound like a 'noob' question, and I know it's not specific to ProcessWire, but I felt like asking, so I know better ;-)

Thanks!

PS : I've just noticed there was a 'dev forum' for such questions! It should go there... But I guess that's too late... Sorry!

Edited September 11, 2013 by celfred

[diogo]

Any of those methods is valid. The only thing that you are doing wrong, either for readability or performance, is getting the page lot's of times "$pages->get($taskId)". So, if I would change your first block of code it would look like this:

$taskId = (int) $input->post->task[$checked_player]; // extra tip. The (int) makes sure you get an integer
$myPage = $pages->get($taskId); // get the page only once

if ( $myPage->id ) { // if page exists
    // Get the task bonuses
    $task = $myPage->title;
    $new_XP = $myPage->XP;
    $new_HP = $myPage->HP;
    $new_GC = $myPage->GC;
    // Do the math
    $player->XP = $player->XP + $new_XP;
    $player->HP = $player->HP + $new_HP;
    $player->GC = $player->GC + $new_GC;
}
 

Or, and maybe now it makes more sense:

$taskId = (int) $input->post->task[$checked_player];
$myPage = $pages->get($taskId);

if ( $myPage->id ) {
    // Get the task bonuses
    $task = $myPage->title; // <- do you need this here?
    // Do the math
    $player->XP = $player->XP + $myPage->XP;
    $player->HP = $player->HP + $myPage->HP;
    $player->GC = $player->GC + $myPage->GC;
}
 

 

Edit: I left this in the "getting started" forum because you have some PW specific issues there.

[celfred]

Nice explanation. Thank you very much!