Passing arrays between functions

[TechnoDiver]

I'm making a registration form and I'm really new at it. I've had a look about at similar issues but feel like I've got my code set up right, I obviously don't.

I have a registration_handler.php and a functions.php file. I'm simply trying to pass an array ($errors[]) from the functions that generate them to a function that prints them out on registration_handler.php. It seems so easy, but I'm not getting it. Here's my code:

from functions.php: (this is just an example of one of the functions)

// USERNAME (MANDATORY)
  //uses the clean_names() and checks for empty input
  function assign_username() {
    global $conn;
    $username = $_POST['username'];

    if(!$username){
      $errors[] = "A username is mandatory";
      echo "A username is mandatory <br>";

    } else {
      $username = clean_names($username);
    }

    //check for duplicate username and return $username if doesn't exist already
    $username_check = mysqli_query($conn, "SELECT username FROM users WHERE username='$username'");
    $number_rows = mysqli_num_rows($username_check);

    if($number_rows > 0) {
      $errors[] =  "Username already in use <br>";
      echo "Username already in use <br>";

    } else {
      return $username;
    }

    return $errors;
  } // END USERNAME

Also from functions.php, the function to print out the error array:

//display error array
  function show_errors($errors) {

    print_r($errors);

  }

and the function call from registration_handler.php:

if(isset($_POST['register_button'])) {
    
    //assign clean form variables
    $firstname = assign_firstname();
    $lastname = assign_lastname();
    $username = assign_username();
    $email = assign_email();
    $password = assign_password();

    $date = date("Y-m-d");

    $errors = array();
    show_errors($errors);


  }

I'm intentionally leaving the username out to test this on the form but I only get returned

Array()

It echos "Username is mandatory" but it seems like it's not passing the error string to the error array

Can anyone point me towards what I'm doing wrong, please. I appreciate the replies

Edited June 11, 2021 by TechnoDiver

[Barand]

You seem to be in two minds about what your assign_*() functions should return. The one you show can return either a username or an error.

It would be better if it were to return the username or false if there was an error and put the error into the error array during the function's execution, as in this example

    function assign_username(&$all_errors)       /// pass the array by reference
    {
        global $conn;
        $username = $_POST['username'];

        $errors = [];
        if(!$username){
          $errors[] = "A username is mandatory";

        } else {
          $username = clean_names($username);
        }

        //check for duplicate username and return $username if doesn't exist already
        $username_check = mysqli_query($conn, "SELECT username FROM users WHERE username='$username'");
        $number_rows = mysqli_num_rows($username_check);

        if($number_rows > 0) {
          $errors[] =  "Username already in use <br>";
        }
        
        if ($errors) {
            array_merge($all_errors, $errors);
            return false;
        }
        else return $username;
        
    } // END USERNAME
  
  
// CALL THE FUNCTIONS
    $errors = [];
    $username = assign_username($errors);
    $lastname = assign_lastname($errors);
    $username = assign_username($errors);
    $email = assign_email($errors);
    
    show_errors($errors);

 

[mac_gyver]

the main issue is due to local variable scope within functions (the $errors array you are returning is being created anew inside each function call) and wrong-responsibility of function code. user written functions should do useful things (building blocks) that help you to create applications (they should not contain your application specific code), they should accept all input data as call-time parameters, and they should return the result to the calling code. in short, breaking up and wrapping your main code inside of function definitions, then needing to make everything global to get it to work, is just adding a lot of typing that doesn't add any value to what you are doing.

also, buried within this wall of code is a logic mistake (the looks like it came from w3schools) -

    if(!$username){
      $errors[] = "A username is mandatory";
      echo "A username is mandatory <br>";

    } else {
      $username = clean_names($username);
    }

that code is first testing if the raw value is false or not, then it is using whatever the clean_names() function does to the value throughout the rest of the code. based on what w3schools code has done in the past, this will let a value consisting of all white-space characters pass the raw test, but will use an empty string in the rest of the code. the only modification you should make to user submitted data is to trim it, first, then use that trimmed value throughout all the rest of the code.

as to testing if a username (or any other unique value) already exists, define the column as a unique index, just attempt to insert the data, then detect if a duplicate index error occurred. by first trying to select the data, there's a time gap where dozens of queries can be executed which could then take the value that you think is not in use.

you should also be using a prepared query when supplying external, unknown, dynamic values to a query when it gets executed.

[TechnoDiver]

Barand, I've been reading about referencing variables and I understand it in principle but maybe not entirely in practice. Not sure I understand the use of it here.

I have a few questions if you don't mind - 1) errors[] needs to be assigned within every function that uses it AND in the registration_handler.php  as well as be referenced in the parameters? I understand the last 2, but didn't think it needed assigning in each function too

2) What is internally happening with a program when it runs the return false; in the if statement?

3) Would you mind giving me a ELI5 breakdown of why referencing the variable is done here and how that works throughout the rest of the program, only if you have the time and will, no pressure there.

I'm getting the same result with your code as I was with how I was doing it (just getting Array() as output with no contents) and I thought that once I got more comfortable with the above mentioned issues I'd be able to figure it out. Thanks again

[Barand]

Here's a slightly simpler exampleof achieving same thing

$errors = [];                       // create empty array to accumulate all errors

$a = functionA($errors);            //   call the functions
$b = functionB($errors);            //   passing the error array
$c = functionC($errors);            //   so they can add to it

if ($errors) {
    show_errors($errors);
}
else {
    echo "$a - $b - $c<br>";
}
                                        // normally when passing a variable to a function
                                        // the value is copied to the variable used by the function.
                                        // In this case we pass the array *by reference*
                                        // so that the original array is used and not a copy.
function functionA(&$all_errors)        // that way we can accumulate all the errors in the one array
{
    $hasErrs = 0;
    if (rand(1, 3)==1) {
        $all_errors[] = 'A error';    // write any error messages into the error array
        $hasErrs = 1;
    }
    
    if ($hasErrs) {                             // if there are errors
        return false;                           // a common convention is to return false when a function fails with errors
    }
    return 1;
}

function functionB(&$all_errors)
{
    $hasErrs = 0;
    if (rand(1, 3)==2) {
        $all_errors[] = 'B error';    
        $hasErrs = 1;
    }
    
    if ($hasErrs) {                           
        return false;                       
    }
    return 2;
}

function functionC(&$all_errors)
{
    $hasErrs = 0;
    if (rand(1, 3)==3) {
        $all_errors[] = 'C error';
        $hasErrs = 1;
    }   
    
    if ($hasErrs) {                           
        return false;                         
    }
    return 3;
}

function show_errors($errors)
{
    echo "You had errors<ul>";
    foreach ($errors as $e) {
        echo "<li>$e</li>";
    }
    echo "</ul>";
}

 

[TechnoDiver]

Quote

Here's a slightly simpler exampleof achieving same thing

I had a good chuckle at this. From MY perspective it doesn't seem simpler. I've copied it to analyze it later and I've compared it to your previous example.

I've looked over my amended code, based on your first example, and I completely understand it now, thanks for the new example. The reason to reference the variable is more clear now too.

I'm working through a tutorial that teaches PHP through a CMS project. I spend some time on the tut then try to transfer that knowledge to a personal project I'm working on, the one this is the eventual registration form for. I've edited my code based on the concepts you introduced me to in your first example and have this:

in functions.php -

// USERNAME (MANDATORY)
  //uses the clean_names() and checks for empty input
  function assign_username(&$all_errors) {
    global $conn;
    $username = $_POST['username'];
    $errors = [];

    if($username) {
      $username = clean_names($username);			//i've tried this in the negative too "if(!$username)...."
      

    } else {
      $errors[] = "A username is mandatory";
      echo "A username is mandatory <br>";
    }

    //check for duplicate username and return $username if doesn't exist already
    $username_check = mysqli_query($conn, "SELECT username FROM users WHERE username='$username'");
    $number_rows = mysqli_num_rows($username_check);

    if($number_rows > 0) {
      $errors[] =  "Username already in use <br>";
      echo "Username already in use <br>";
    }

    if($errors) {
      array_merge($errors, $all_errors);  //i've tried this with $errors = array_merge(....) and still didn't work
      return false;						//if I use echo ".." it echos but shows an empty array

    } else {
      return $username;
    }
    
  } // END USERNAME

and

//display error array
  function show_errors($errors) {
    print_r($errors);
  }

and registration_handler.php looks like this:

if(isset($_POST['register_button'])) {
    
    $errors = array();
    
    //assign clean form variables
    $firstname = assign_firstname();
    $lastname = assign_lastname();
    $username = assign_username($errors);
    $email = assign_email();
    $password = assign_password();

    $date = date("Y-m-d");

    
    show_errors($errors);

    echo "<br>";

    echo "First Name: " . $firstname . "<br>";
    echo "Last Name: " . $lastname . "<br>";
    echo "Username: " . $username . "<br>";

  }

Once I start feeling more comfortable with PHP I run into a problem like this. I obviously still need some work transferring data between functions and other files, I thought I understood it, this code looks functional to me but yet it still shows Array() as the output without content. I've tried an 'echo' statement everywhere something is supposed to happen and always get the echo, so that part of the code is running, but "Username is Mandatory" is not being inserted into the $errors[] array. What more is there?

I make a mental checklist of how I've come to understand how the data moves -> assigned array in both the file and the function, set the function parameter, added the string to the array, sent the array to a print function and called the print function in the other file. I can't fathom what more could possibly need to be done, obviously there's something, but I'm not seeing it. What have I missed?

[Barand]

My apologies (blame it on age - I guess I had a "senior" moment).

array_merge($all_errors, $errors);

should be

$all_errors = array_merge($all_errors, $errors);

 

[TechnoDiver]

That just shot to shit everything I thought I understood about references in this instance. It also taught me to try everything and not assume something won't work because I can't understand how it does.

I tried $errors = array_merge(.......); and didn't try $all_errors I just didn't see how it could work that way.

Thank you, you've been a great help, but tell me if you don't mind - why $all_errors here? I can't contextualize it working here at all

[TechnoDiver]

so I didn't want to start a new thread for this and I couldn't append it to my last post. I'm not trying to be too clever with the program but I decided to try something to help me understand more how data moves between functions.

I took the code you've been helping me with and made a function from it ->

function check_errors(&$all_errors, $field) {
    $errors = [];
    if($errors) {
      $all_errors = array_merge($errors, $all_errors);
      return false;

    } else {
      return $field;
    }
  }

it's an internal function to be used in the assign_username, email and password functions.

check_errors($errors, $password); //or check_errors($errors, $username) etc

And this is another example of just when I started feeling comfortable with something I prove myself wrong. I'm finding it really frustrating because I feel that I can picture the data moving through the functions but why wouldn't the above work? I feel there's a simple concept that I'm missing or overlooking.

I hate to be the guy who keeps asking about the same thing over and over, but I really want to understand how the data moves in a web app and I feel a good way is to let people respond however than process it all later into something cohesive. That one person may just leave a response that suddenly flicks the light bulb on

Saying all that. What is wrong with this function, please? I'm really not grasping why it's not returning the error string (it's the empty array again).

[Strider64]

Personally, I just write functions that I try to do one thing and send it back to the main program. Here's an example of what I'm talking about

        $query = "SELECT username FROM " . static::$table ." WHERE username = :username";
        $stmt = Database::pdo()->prepare($query);
        $stmt->execute(['username' => $username]);
        if ($stmt->fetch(PDO::FETCH_ASSOC)) {
            $data['check'] = true;
            return $data;
        }

        $data['check'] = false;
        return $data;

I check the username against the database table then send and array back. Yes, I know it only has one index in the array, but then I can use that for other code like a validation array for example? This might help or it might not? I just find it easier not to have too many things going on at once.

Edited June 13, 2021 by Strider64

[Barand]

12 hours ago, TechnoDiver said:

What is wrong with this function, please?

It isn't doing any checking.

You create, in the function, an empty local $error array, then your code says "if this (empty) array is not empty, add the empty array to $all_errors array.

You should be

create empty $error array
check for error
if error
   add message to error array
endif

   // repeat for other other error checks as necessary

if error array is not empty
   add it to the all_errors array
   return false
endif
return field

 

Edited June 13, 2021 by Barand

[TechnoDiver]

rookie error, thanks

[TechnoDiver]

On 6/13/2021 at 5:21 AM, Strider64 said:

Personally, I just write functions that I try to do one thing and send it back to the main program. Here's an example of what I'm talking about

        $query = "SELECT username FROM " . static::$table ." WHERE username = :username";
        $stmt = Database::pdo()->prepare($query);
        $stmt->execute(['username' => $username]);
        if ($stmt->fetch(PDO::FETCH_ASSOC)) {
            $data['check'] = true;
            return $data;
        }

        $data['check'] = false;
        return $data;

I check the username against the database table then send and array back. Yes, I know it only has one index in the array, but then I can use that for other code like a validation array for example? This might help or it might not? I just find it easier not to have too many things going on at once.

Thank you for your response, I didn't notice it until now. I'm still decyphering it

 

On 6/13/2021 at 6:43 AM, Barand said:

It isn't doing any checking.

You create, in the function, an empty local $error array, then your code says "if this (empty) array is not empty, add the empty array to $all_errors array.

 

Barand, I had to work on something else for a few days so I'm just getting back to this.

I saw the mistake with assigning an empty array and the checking if it were empty immediately afterwards. That was certainly a rookie mistake.

But there's still something not clicking.

1. I checked for errors in the assign_field() function (in this case $password)

2. I called the check_errors function with the right parameters.

3. The function check_errors() checks for an empty array, merges them with the others if they exist or returns the $field if everything is ok.

I'm really baffled. I'm not sure what else you were suggesting I need to do.

Here's the code as it stands now:

  // PASSWORD (MANDATORY)
  //validate, confirm assign password
  function assign_password() {
    $password = clean_password($_POST['password']);
    $password2 = clean_password($_POST['password2']);
    $errors = [];

    if($password == $password2) {
        
      //password 6-30 characters long
      if (strlen($password) < 6 || strlen($password) > 30) {
      
        $errors[] = "Your password must be 6-30 characters long";
      }

      // FOR DISPLAY PURPOSES, THIS WILL UNDERGO VAST CHANGES EVENTUALLY
      //password is alpha-numeric
      if (!ctype_alnum($password)) {
        
        $errors[] = "Your password must be English characters and/or numbers";
      }
      
    } else {
      $errors[] = "Your passwords don't match";
      echo "Your passwords don't match";
    }

    check_errors($errors, $password);

  } // END PASSWORD

and check_errors:

//this function appears in username, email and password_assign()
  //it simply checks for any content in the error array and assigns the field if no errors
  function check_errors(&$all_errors, $field) {
    if(!empty($errors)) {
      $all_errors = array_merge($errors, $all_errors);
      return false;

    } else {
      return $field;
    }
  }

I've gone over it in my head dozens of times, tried mentally tracing exactly what the data was doing on each step, put it away for awhile to study other areas of PHP hoping for an AH HA! moment, read through your 'pseudo-code' in your last comment, went to Edabit to practice with functions. I still can't see what's wrong here, and I'm sure it's something basic. Is it a failure in my logic? My implementation?

Edited June 15, 2021 by TechnoDiver

[Barand]

When you call check_errors() from within your functions you are passing the local error array that was created inside the function and not the main error array in which you want to accumulate all errors. So it just accumulates them in that local array and they get lost when the function finishes.