Jump to content
Sign in to follow this  
celfred

Coding best practice?

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

Share this post


Link to post
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

Share this post


Link to post
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
Sign in to follow this  

  • Recently Browsing   0 members

    No registered users viewing this page.

  • Similar Content

    • By Marvin
      Hello, i want to ask, i maintain a website that using a processwire and php, and i want to make an archive at my website using a subfolder system, but when i try,
      the sebfolder is show but when i click the files in that subfolder not show, and my browser just show me an error Invalid argument supplied for foreach(), i don't know why it error
      Here i attach my code and my screenshoot website :
      This is my code

      This is result of my website

      This is my error

       
       
    • By Mithlesh
      Hi there,
      My form is not getting submitted, it is showing:
      Unable to verify successful email delivery of this form submission.
      Attaching for your reference as well: 

      In the Backend, it is showing Connection timed out with smtp.gmail.com
      Pl guide me how to resolve that
    • By CareerTeam GmbH
      Hi there,
      We are an executive search agency based in Germany looking for a freelancer (2-5 days per week) supporting us with the development and design of our websites. The position will be located in Hamburg, Germany and it would be great if you are on short call. German language knowledge is mandatory. 
      You can reach me via email jobs@careerteam.de.
      Thank you!
      Regards
      Annemie
    • By louisstephens
      I was really unsure of how to actually title this post, so I do apologize (if someone has a better idea, I will gladly edit it). I am using the profields: pagetable field to allow people to create their own "content" (copy, image, button, etc etc) and rearrange it. I also included a field called "column_size" using the RangeSlider set to (1-12).
      I guess I'll clarify a bit more on this. I am using flexbox where the "row" is <section></section> and the columns are <div class="column"></div> have given the "columns"  flex: 1 1 0; so no matter how many columns you have, the columns will automatically adjust for new content. Where my confusion is coming in: If a user has set up 3 copy items (with 12, 5, 7 respectfully for the column_size), how do I actually output this in my template? I was going to use a switch statement to handle the various items which I thought made it quite easy, but with closing sections and columns I have confused myself as I assume I need an if statement to check if the column size is > 12, or = 12 to determine the actual closing/opening of sections. I apologize if I have not made this very clear. I am a bit unsure how to word this let alone to go about this. 
      Im very appreciative of for any insight into this.
       
       
    • By louisstephens
      So I have been hard at work creating url segments for a template (api) and everything is working swimmingly in creating a simple end point for svelte.js. I have however, run into a few questions that I can wrap my head around.
      In my api template I have:
      if($input->urlSegment1 === 'clients') { header('Content-Type: application/json'); $clients = $pages->find("template=clients"); $client_array = array(); foreach ($clients as $client) { $id = $client->id; $title = $client->title; $url = $client->url; $clientName = $client->client_name; $clientColor = $client->client_color->value; $assigned = $client->assigned_to->user_full_name; $client_array[] = array( 'id' => $id, 'code' => $title, 'name' => $clientName, 'associated_users' => $assigned, 'url' => $url ); } $client_json = json_encode($client_array, true); echo $client_json; } The output json from this is:
      [ { "id":1644, "code":"abc", "name":"Test Name", "associated_users":null, "url":"\/pw\/clients\/abc\/" }, { "id": 1645, "code": "xyz", "name": "Test Name", "associated_users": null, "url": "\/pw\/clients\/xyz\/" }, ] I was curious is it possible to add in "clients" before this output json so it would appear as 
      clients: [ { "id":1644, "code":"abc", "name":"Test Name", "associated_users":null, "url":"\/pw\/clients\/abc\/" }, { "id": 1645, "code": "xyz", "name": "Test Name", "associated_users": null, "url": "\/pw\/clients\/xyz\/" }, ] I was not really sure of how to tackle this in my php code, and have spent more time than I care to admit trying to figure it out. Another question I have is that "associated_users" is returning null, which in this instance is correct. It is a multi page field that is set to pull a custom name field from the users template, ie "Louis Stephens" would be associated with the first page. I understand that I need to use a foreach to get the correct data, but I was really unsure of how to place this inside an array, or update the array with the new data. Any help with any of this would greatly be appreciated.
×
×
  • Create New...