Jump to content

Coding best practice?


celfred
 Share

Recommended Posts

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 by celfred
Link to comment
Share on other sites

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.

  • Like 6
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

×
×
  • Create New...