Questions about a Function inside a While loop...

[Jim R]

I'm using this line a lot on my site.  It would be nice if I didn't have to change it 10 times over when I decide to change something.  Can I wrap that into a Function and use it in a While loop?

echo '<div><a href="/tag/'. strtolower($nameFirst) . '-' . strtolower($nameLast) .'">'. $nameFirst . ' ' . $nameLast .'</a>, ';

I have this above it in the While loop.  

$nameFirst = $row['nameFirst'];
$nameLast = $row['nameLast'];

 

My function predictably looks like this...

function player_name () {

echo '<div><a href="/tag/'. strtolower($nameFirst) . '-' . strtolower($nameLast) .'">'. $nameFirst . ' ' . $nameLast .'</a>, ';

}

 

[ginerjm]

Maybe I don't understand but I don't see why not.  Use unique var names inside the function and pass in values for them via the headers.

	function player_name ($fn, $ln)
	{
	    echo '<div><a href="/tag/'. strtolower($fn) . '-' . strtolower($ln) .'">'. $fn . ' ' . $ln .'</a>, ';
	    return;
	}
	

You are missing the end div tag - is that intentionally?  Perhaps you don't even want to start the div in the function?

[Jim R]

I did not want the DIV in there.  I removed it.  No change.  

It looks like the function isn't getting the data into it.  I tried putting the variables between the ( ), but that just threw an error.  

 

Quote

Too few arguments to function player_name(), 0 passed in /home2/csi/public_html/wp-content/plugins/csi_reviews.php on line 182 and exactly 2 expected

 

[ginerjm]

You don't code much, do you?

The function is as I wrote it.  When you want to use it you do:

	player_name($curr_fname, $curr_lname);
	

The values in these 2 vars will be passed into the function and be referenced as $fn and $ln

[maxxd]

The error means you're not passing any variables to the function. Once you add $fn and $ln to the function definition you need to actually pass the first name and last name when you call that function.

[Jim R]

4 minutes ago, ginerjm said:

You don't code much, do you?

 

Ehhh...I'm self taught, and it's a hobby.  I run into learning curves.

[Jim R]

I didn't have the variables in both where I created the functions and where I used it.  

 

Thank you!

[ginerjm]

You mean "learning bumps".

[ginerjm]

One last thing.  Your topic heading say "inside a while loop".  I do hope you know enough to not place the actual function inside the while loop.  You will be "calling" it From the while loop, but you don't want it written there.  IMHO functions are to be placed outside of the main line of your php code.  Burying them inside the logic just interrupts one's view of the code when trying to read thru it.  Your's is a one line function but when you actually have need to write a real one with many lines you'll see that you don't want to place it in the way of your thought process.  Using a function is one of the best reasons to clean up your main logic and get some lines of code out of sight.

[Jim R]

13 minutes ago, ginerjm said:

One last thing.  Your topic heading say "inside a while loop".  I do hope you know enough to not place the actual function inside the while loop.  You will be "calling" it From the while loop, but you don't want it written there.  IMHO functions are to be placed outside of the main line of your php code.  Burying them inside the logic just interrupts one's view of the code when trying to read thru it.  Your's is a one line function but when you actually have need to write a real one with many lines you'll see that you don't want to place it in the way of your thought process.  Using a function is one of the best reasons to clean up your main logic and get some lines of code out of sight.

The function itself is in a separate file.  There are four things I''m using about 10 times over, and a couple of them I tinker with.  Every time I make a change, I have to alter all instances.  

 

[ginerjm]

YOu have to make your functions flexible enough so that you don't have to tinker with them - just alter the arguments coming in.

[gizmola]

Hey Jim!

First a tip.  You will be better off if you attempt to adopt PHP Standard coding conventions.  I will suggest this link:  https://symfony.com/doc/current/contributing/code/standards.html

Most of these standards are in PSR-1 and PSR-12, which were ratified and adopted as community standards.

The main one you ran afoul of with this plan is the name of your function.  Don't use underscores.  Instead use "camel case" for naming.  Since you are constructing a standard player name tag, a good name for this function might be "formatPlayerTag" or "makePlayerTag" or "getPlayerTag".  Often with databases, there are "getters" that will start with the word "get", so in this case you might not want to use get, but something that better indicates that you are making a standardized string of some sort from other data.

Another thing you want to avoid is mixing markup with data.  I know it seems like an easy way to go right now, but I would advise against spitting out the html in this function -- only have it output the text portion.  Another thing you don't want to do here is have a function that does an "echo".  Functions should take parameters, make computations and return a result, exactly as math functions do.

My suggestion:

function formatPlayerTag($nameFirst, $nameLast) {
  return strtolower($nameFirst) . '-' . strtolower($nameLast);
}


#used in the while

echo '<div><a href="/tag/' . formatPlayerTag($nameFirst, $nameLast) . '">' . $nameFirst . ' ' . $nameLast . '</a>';

 

[Jim R]

33 minutes ago, gizmola said:

Hey Jim!

First a tip.  You will be better off if you attempt to adopt PHP Standard coding conventions.  I will suggest this link:  https://symfony.com/doc/current/contributing/code/standards.html

Most of these standards are in PSR-1 and PSR-12, which were ratified and adopted as community standards.

The main one you ran afoul of with this plan is the name of your function.  Don't use underscores.  Instead use "camel case" for naming.  Since you are constructing a standard player name tag, a good name for this function might be "formatPlayerTag" or "makePlayerTag" or "getPlayerTag".  Often with databases, there are "getters" that will start with the word "get", so in this case you might not want to use get, but something that better indicates that you are making a standardized string of some sort from other data.

Another thing you want to avoid is mixing markup with data.  I know it seems like an easy way to go right now, but I would advise against spitting out the html in this function -- only have it output the text portion.  Another thing you don't want to do here is have a function that does an "echo".  Functions should take parameters, make computations and return a result, exactly as math functions do.

My suggestion:

function formatPlayerTag() {
  return strtolower($nameFirst) . '-' . strtolower($nameLast);
}


#used in the while

echo '<div><a href="/tag/' . formatPlayerTag($nameFirst, $nameLast) . '">' . $nameFirst . ' ' . $nameLast . '</a>';

 

I appreciate that, but I'm trying to avoid typing all of that in the future, especially in regards to a player's name. 

[gizmola]

The answer to that is to use "templating".  PHP is designed as a template language but most people use a template component library like smarty or twig.  I would highly recommend twig.  

Either way you seperate your presentation from your logic.  It's been a long time since I used Smarty, but with twig, you have the ability to use blocks and partials, so you can have a partial block that you include whenever you want a particular block of html+ data.  This would solve your complaint in regards to reuse of markup.

So how could you make this all work just with PHP?

You make a small include file that looks like this:

<a href="/tag/<?php formatPlayerTag($nameFirst, $nameLast); ?>"><?= $nameFirst ?> <?= $nameLast ?></a>;

You might name this script using a convention like '_player_name_href.php';

In your main script where you have your output, and the link is meant to appear you just include it:

// Read all data into an array using whatever database routines you are using

$players = some_fetch_all();

foreach ($players as $player) {
  $nameFirst = $player['nameFirst'];
  $nameLast = $player['nameLast'];
  include('_player_name_href.php');
}

This will create your list of links, where the creation of a tag is in a single function (you would want to have in a library script you include) AND your html snippet is in a single place so that if you change it you'll change it in the one place.   

You've done relatively modern functional programming, and you've kept separation of concerns, started to unmix logic and presentation so that you aren't producing spaghetti code.

Just to tie a bow on this discussion, let's say you don't want to try your hand at making your own templating as I illustrated.  You can stay with the functional programming approach, by simply creating a 2nd function that outputs your markup.  This addresses several of your complaints without making a gobbledy gook function that mixes the 2. Again you will use your original formatPlayerTag function, but you'll pass the data to a 2nd presentation function. Here's how you would do it:

 

function formatPlayerTag($nameFirst, $nameLast) {
  return strtolower($nameFirst) . '-' . strtolower($nameLast);
}

function getPlayerNameWithTag($nameFirst, $nameLast) { 
    return '<div><a href="/tag/' . formatPlayerTag($nameFirst, $nameLast) . '">' . $nameFirst . ' ' . $nameLast . '</a>';
}

Now to use this:

echo getPlayerNameWithTag($row['nameFirst'], $row['nameLast']);

You stick both of those routines into a library script you make, and include it in any scripts where you need to create the links, and you have a standard format, with 2 simple functions you would need to alter (should that be required).   The more that you can have functions which do a single thing and return a result, the more robust and testable your application will be.  

 

 

[Phi11W]

On 3/23/2020 at 1:45 AM, Jim R said:

I appreciate that, but I'm trying to avoid typing all of that in the future, especially in regards to a player's name. 

An alternative is to encapsulate this logic into a Class that represents a Player.

You would populate an instance of this class from a database query and the class contains a method that handles this name formatting for you - with the advantage that it willthen be consistent for any Player. 

class Player
{
   function __construct( ... ) // Probably from a database query 
   {
      $this->forename_ = ... ; 
      $this->surname_ = ... ; 
   }
   
   function __toString() : string // Simplifies debugging 
   {
      return $this->formatName(); 
   }
   
   // Here's the encapsulation of the name-formatting logic
   // Write it once, use it many times! 
   public function formatName() : string 
   {
      return sprintf( '%s-%s', $this->forename_, $this->surname_ ); 
   }
   
   public function forename() : string // Simple property retrieval 
   {
      return $this->forename_ ; 
   }
   
   public function surname() : string // Simple property retrieval 
   {
      return $this->surname_ ; 
   }
   
   private string forename_ ; 
   private string surname_ ; 
}

 

Regards, 
   Phill  W.

[gizmola]

Hi Phi11w!

I agree strongly that a model class for player would be a great addition.  If you look closely at what was requested, you might notice that JIm R has implemented linking based on the user name, so it's not quite as simple as making a name out of first/last, although you would improve the part of the code that utilizes the firstname,lastname in links.

I can tell that we both would probably agree that adopting some sort of MVC is a best practice, and you illustrate a very easy way to start doing that without full scale adoption of Symfony, Laravel or some other framework.