Array not being passed to function

[SaranacLake]

In my main script I set $errorArray with values when required survey questions go unanswered.

The ne code that checks for missing required values is working perfectly, and right before I pass $errorArray to a function which rebuilds the survey questions - this time with the error messages - I do a var_dump($errorArray) and that array has errors in it as expected.

But while my function that re-builds the survey questions gets called and does indeed re-build the survey questions, the $errorArray isn't getting pased for some strange reason?

(I'm not sure if i have ever passed an array to a function before.)

In my function I have...

	function generateSurveyItem($dbc, $questionNo, $questionID, $surveyResponseArray=NULL, $errorArray=NULL){
	 
	}
	

 

When I var_dump($errorArray) at the top of the function, and submit a blank form, I end up with a blank page and a file path to my PHP script ending with...

	/public_html/utilities/function.php:3010:null
	

 

Questions:

1.) What is happening?

 

2.) I thought =NULL in the parameter list just meant that the argument is "optional"?

 

3.) I am able to successfully pass $surveyResponseArray to this function, so what is up with $errorArray?

 

P.S.  When I run things, and submit an empty form - thus no "required" fields get completed - the survey gets re-displays, and any values chosen are "sticky", but my error messages should also be displayed to let the user know what is mising, and that is the part that isn't working?

This has to be something silly stupid to fix...

 

Edited February 16, 2021 by SaranacLake

[Barand]

What's your code that calls generateSurveyItem() ?

Edited February 16, 2021 by Barand

[SaranacLake]

7 minutes ago, Barand said:

What's your code that calls generateSurveyItem() ?

 

Here is a snippet

if (empty($errorArray)){
    // Write to database
    
}else{
    // Send $errorArray to generateSurveyItem() to re-build survey items with error messages.
    $questionNo =1;
    
    while(mysqli_stmt_fetch($stmt2)){
        $surveyItemArray[$questionID] = generateSurveyItem($dbc, $questionNo, $questionID, $surveyResponseArray, $errorArray);
        
        $questionNo = $questionNo + 1;
    }
}
	

xx

..

 

Edited February 16, 2021 by SaranacLake

[Barand]

You send and receive the array OK.

That just leaves your processing of it inside the function.

[SaranacLake]

12 minutes ago, Barand said:

You send and receive the array OK.

That just leaves your processing of it inside the function.

I don't understand your response.

In my main script, I did a var_dump($errorArray) and I get values if I submit the form with no values or without required fields being answered.

But inside and at the top of the function, I do another var_dump($errorArray) and PHP is showing.

	/public_html/utilities/function.php:3010:null
	

 

So clearly something is happening to the data in $errorArray from when it is passed to my function and then inside the function.

[gizmola]

Defining a function with optional parameters is done the way you did it.

 

<?php

function foo($bar, $fruit=null, $candy=null) {
    echo "bar: $bar" . PHP_EOL;
    echo "fruit: $fruit" . PHP_EOL;
    echo "candy: $candy" . PHP_EOL;
}

foo('one bar');
foo('one bar', 'apple');
foo('one bar', null, 'snickers');

 

With basic functions, all parameters are passed by value, not pass by reference.

$error = array('message' => '');

function testparams($msg, $error) {
    $error['message'] = $msg;
} 

testparams('BAD SYSTEM', $error);

// Original error array was not changed, because it was copied internal to the function
echo "Error: {$error['message']}" . PHP_EOL;

 

To get around this, you can tell PHP that the $error array should be passed by reference, and change the original array variable.

 

$error = array('message' => '');

function testparams($msg, &$error) {
    $error['message'] = $msg;
} 

testparams('BAD SYSTEM', $error);

echo "Error: {$error['message']}" . PHP_EOL;
// Result is Error: BAD SYSTEM

 

[SaranacLake]

@gizmola,

Thanks for the demo.  I am familiar with passing by value vs by reference, but your example is helpful.

The problem, however, is not that, because I am using $errorArray within the function - which re-generates each survey question.  (If I was referencing $errorArray outside of the function, and I needed the $errorArray to be updated based on the function, then your example would apply.)

One dumb thing I was doing that caused one issue, is that I had

	var_dump($errorArray);
	exit();
	

turned on inside my array, but that created the following issue...

 

The first time I call $generateSurveyItem( ), I need it to execute in full so that my survey page gets painted.  (That is why i was seeing NULL, because my test code above stopped the page from being painted!)

 

However, if I comment out those two lines, load the survey page, then uncomment those two line, and submit the form, then I get a var_dump( ) displaying all required fields left blank.

So my $errorArray *IS* getting populated by my new check for required fields code, and $errorArray *IS* getting passed properly to my function.

Now the question is, "Why aren't my survey questions getting appended with the error message from $errorArray??

Gotta look at my function code one more time...  

 

 

 

[gizmola]

We don't have enough of your code to look at but to be clear -- when passing by value, you can update the array inside the function.  You can echo out values, make assignments etc.  and everything will look fine.  Any of those changes will be discarded once the function exits, however, as inside the function you are working with a copy of the original variable as it was when the function was invoked.  

If this is not the issue, then we need to see some actual code, to help further, as anything at this point is simply an educated guess.

[SaranacLake]

3 minutes ago, gizmola said:

We don't have enough of your code to look at but to be clear -- when passing by value, you can update the array inside the function.  You can echo out values, make assignments etc.  and everything will look fine.  Any of those changes will be discarded once the function exits, however, as inside the function you are working with a copy of the original variable as it was when the function was invoked.  

If this is not the issue, then we need to see some actual code, to help further, as anything at this point is simply an educated guess.

Points duly noted.

Thanks to @maxxd suggestion last night, I think I am over the hump on things, and I think I know where the problem is, but there is so much code to sort through...

Let me get back to you on this, but i don't think using "by reference" will help here, although thanks for the lesson!  🙂

******************

And THAT is what I hope to accomplish in v2.0 - even more than Git @gizmola 😉 - is doing a better job of compartmentalizing my code.  It is such a b*tch reading through thousands of lines of code to fix something like I am on now.  I forgot how involved my survey actually is?!  I think that using MVC and getting smarter with functions, and likely using OOP will make it easier to work with my code.  But who knows?!

Right now my website is between 50,000-60,000 lines of code and that is A LOT to me.  I also have single script that go over 3,000 lines, so scrolling up and down and up and down looking at IF-THEN-ELSE statements on a tiny laptop can be overwhelming!  And for this current issue, it means going back and forth between multiple scripts, so by the time I get to functions.php, I have forgotten what i was looking at in xyz.php.

I guess even the best written code struggles from such complexities to a point, but THIS is what iw ant to get better at so i can code better and faster.

 

[gizmola]

I haven't seen a lot of your code, but the other thing that jumps out is that having functions where you are passing 4-5 parameters indicates that you probably have blocks of code that are trying to do too many things, and are not discrete enough.  I certainly could look back at code I wrote in the past and admit I did the same thing on many occasions.  This leads to the type of problems you are concerned about:  large code base, concerns over side effects if you make a change, cascading throughout the code, lack of DRYness etc.

This tends to happen when you build everything from scratch, as your code evolves into a thing "that works for what I have" rather than something designed from the outset to be clean and maintainable, and unit testable.  

While you can't go back and change large swaths of the system now, you certainly can improve it incrementally by writing smaller more discrete functions for anything new or currently undergoing modification, and refactoring whenever possible.  Sometimes a small improvement can be game changing in terms of system quality and maintainability.

• Write functions that do one thing, and always return a result of the same type.
• Most functions should have a handful of parameters.  If there are more than a few essential parameters, look at ways to break the function into smaller more discrete functions/pieces that you can use to assemble your results

Obviously having functions with 1000's of lines of code is hard to work with.  Most editors have ways to open multiple windows and add bookmarks in your code, when you have to move from one to another.  Some of the fancier IDE's also let you jump back and forth in your code, by creating linkages from the files where classes and functions were defined that the editor can then use to open the appropriate file when needed, akin to clicking on a hyperlink.  PHPStorm has that type of function, and can really help when navigating a large codebase. 

 

[SaranacLake]

7 minutes ago, gizmola said:

I haven't seen a lot of your code, but the other thing that jumps out is that having functions where you are passing 4-5 parameters indicates that you probably have blocks of code that are trying to do too many things, and are not discrete enough.

Not just functions - all of my code.

The way I learned to code is procedurally, so I code things based on what you want to do.  For instance, my "article.php" script - which displays an article and user comments below and all functionality related to that is a little over 3,000 lines of PHP, because I coded some "use-case" as a single script.  (Not the way OOP works.)

 

7 minutes ago, gizmola said:

 I certainly could look back at code I wrote in the past and admit I did the same thing on many occasions.  This leads to the type of problems you are concerned about:  large code base, concerns over side effects if you make a change, cascading throughout the code, lack of DRYness etc.

Bingo!

if you were almost done with a decade long project, would you go in and tweak some section for fear it could cascade and break thousands of lines of code?  (Of course that can apply to any large system, but more so the way i coded things.)

 

7 minutes ago, gizmola said:

This tends to happen when you build everything from scratch, as your code evolves into a thing "that works for what I have" rather than something designed from the outset to be clean and maintainable, and unit testable.  

Well, I disagree here.

My code isn't poorly written, it is just hard to work with since it is a birth-to-death approach!  (I recall reading from some OOP expert that a class should never be more than 100 lines of code.)

I have spent the last few weeks walking through my code base line by line and getting my head back into how it works.

Except for this current script, i would say that all of my code made sense - although some took a long time to load into my brain and turn on a lightbulb! - and not surprisingly to me, I have only had to tweak a *few* lines of code after reading over 40,000 lines so far.

So to an outsider, getting into my code might be scary, i think it reads like a book once you see my coding style. Also, i have my code so heavily commented, most of it is self-explanatory.

Also, I want to point out that the reason this site has taken over a decade is that it IS well thought-out.  (In fact, a little too much so!)

My challenge is to learn how to break a 3,000 line script into 20 components that are shorter, easier to update, and more reusable.

 

7 minutes ago, gizmola said:

While you can't go back and change large swaths of the system now, you certainly can improve it incrementally by writing smaller more discrete functions for anything new or currently undergoing modification, and refactoring whenever possible.

Once I go live, I am going to update any architectural designs I have written out on paper, update them, and then start coding a new v2.0 from scratch.

That way I have a working site, and I can ferret out all of the annoying things that I want fixed but that would take too much energy to do.  (Better to start with a new frame and body versus trying to twist a wreck back into an original!)

 

7 minutes ago, gizmola said:

  Sometimes a small improvement can be game changing in terms of system quality and maintainability.

• Write functions that do one thing, and always return a result of the same type.
• Most functions should have a handful of parameters.  If there are more than a few essential parameters, look at ways to break the function into smaller more discrete functions/pieces that you can use to assemble your results

Most of my functions have 3-4 parameters, so that isn't too bad.  What is bad is how I intermingled "presentation' with "business logic".  Frankly, I think PHP encourages that kind of bad coding, which is why I'd like to get into something more substantial like Python, but time will tell.

 

7 minutes ago, gizmola said:

Obviously having functions with 1000's of lines of code is hard to work with.

Exactly.

What is worse, is that i have a single 'functions.php" script with ALL of my functions and it is like 8,000 - 10,,000 lines of code long?!

In the future, I am thinking "One function, one script" i sthe way to go, as I think that is how OOP works where they have "One class, one file".

 

 

7 minutes ago, gizmola said:

Most editors have ways to open multiple windows and add bookmarks in your code, when you have to move from one to another.  Some of the fancier IDE's also let you jump back and forth in your code, by creating linkages from the files where classes and functions were defined that the editor can then use to open the appropriate file when needed, akin to clicking on a hyperlink.  PHPStorm has that type of function, and can really help when navigating a large codebase. 

I know I need to strengthen my skills in IDE's.  I use NetBeans and am probably only using 10% of its features which is sad.  In fact, I think THAT is mor eimportant than learning Git - although I am planning on doing that.

 

There is also one other thing you left out...

I am 10-15 years older - and more senile - than I was when I started with LAMP, and at my age, those 10-15 years are SIGNIFICANT!!!

I'm not dead yet, but I am also no longer 20-something, and I am starting to "appreciate' how my mind isn't as sharp - nor my stamina as strong - as it was in say 2000.

That is even more reason to get damn good with IDE's, Git, MVC, OOP, and use generally accepted coding 'best practices" ASAP.

In the end, I know a lot more than many people give me credit for, HOWEVER, thee is much I can do to take things "to the next level", and if I want to become a millionnaire, then i better hurry up and start doing those things in v2.0!

 

😀

[kicken]

2 hours ago, SaranacLake said:

I recall reading from some OOP expert that a class should never be more than 100 lines of code.

That's far too restrictive and either something that was misunderstood or came from a crazy person.

I came across a rule of thumb some time ago that I try to live by which goes a little something like this:

1. A function should fit entirely on screen (no vertical scrolling required).  How many lines this is will vary based on resolution/fonts and such, but ends up being around 65 lines for my environment.
2. A single line shouldn't be longer than the width of the screen (no horizontal scrolling required).  That ends up being around 120 characters for me.
3. A function shouldn't have statements nested more than 5 levels deep
4. Use a descriptive name for the function even if it's verbose.

As with many things those are rules of thumb and not everything will conform to them, but I find they are generally pretty easy to follow.  If you end up breaking one of them, then it's time to analyze the situation and decide if you need to break the rule or if it can be refactored.

For rule #1, some things I only consider to be one line because they can be easily code-folded away.  For example I have many functions that are way more than 65 lines, but a lot of those lines are a large SQL statement that I can just fold down to a single line.

Rule #3 helps make rule #2 easier to hit as you don't waste a bunch of screen real estate indenting things a bunch.  Something nested 5 levels with a 4-space indent would be wasting 20 characters on the indentation.

In my experience, refactoring old code to help fit the above rules is relatively simple.  Start by just moving blocks of code, for example move the body of a loop into a new function then call that function from the loop.  Alternatively, just move the entire loop and replace it with a function call.

2 hours ago, SaranacLake said:

What is worse, is that i have a single 'functions.php" script with ALL of my functions and it is like 8,000 - 10,,000 lines of code long?!

In the future, I am thinking "One function, one script" i sthe way to go, as I think that is how OOP works where they have "One class, one file".

Having a bunch of functions in a single file isn't necessarily bad.   It'd be best to at least group them in some way rather than just dump everything in one file.  For example, I have a few functions that deal with manipulating arrays which are all grouped together in a file.  Some other functions that handle validating input are in a separate file.   If you want to do one function, one file like with classes that's fine as well but not generally necessary imo.

 

[SaranacLake]

@kicken

Thanks for the tips!  👍

[maxxd]

I pretty much agree with kicken, but feel the need to throw in my own two cents.

First off, tip #3 is something everyone who codes should live by. To add to it, don't string together a million conditionals - I'm currently dealing with a code base that has a 180-line if-elseif-else tree and it makes me wanna barf. For what is supposed to be a conditional logic operation, this is shockingly devoid of both condition and logic.

Personally, I think any sort of hard and fast numerical limit on function/method/class width or length is kinda manufactured. Use judgment and good formatting. For the width, break across lines when it becomes hard to read. For instance, there's nothing inherently wrong with this code:

if(empty($_POST['firstname']) || empty($_POST['lastname'])){
 // ...
}

Plenty easy to read, but if you've got more validation to do, this becomes a nightmare and should be broken, like so:

if(
		   empty($_POST['firstname'])
		|| empty($_POST['lastname'])
		|| empty($_POST['middlename'])
		|| empty($_POST['fieldicareabout'])
		|| empty($_POST['anotherimportantfield'])
		|| empty($_POST['myrefridgerator'])
	){
 // ...
}

Granted, this board's indenting is far larger than mine in VSCode, but hopefully you get the point.

As far as length, methods should do one thing, and objects should handle one situation. What you've got now is what's called a god object in OOP, and it's - as you're finding - difficult to reason about and maintain. On the other hand, the issue you can run into with one function per file is that it can become a mess of code relationship that's hard to follow if you're not extremely diligent. It also absolutely requires a full test suite because chances are you'll be including the same file in multiple places, and this can lead to hard to find bugs when you forget you use a function somewhere in a different part of the code.

And finally, just to beat the horse some more, format everything appropriately. I've spent untold hours at work trying to parse SQL queries because the original author wrote them out in one long line instead of using line breaks and indenting to make it make sense. Obviously PHP isn't Python or Ruby in that indentation doesn't actually affect the code, but there's a reason the authors of some programming languages chose to use indentation instead of brackets or curly braces - it makes the code easier to read, and illuminates the intention of the functionality.

Wait - I lied; one more thing (it's late but I'm not sleepy and my mind is going, sorry). Absolutely use descriptive and appropriate method and variable names, but document as well. Just because your method is called `applySalesTax` doesn't mean I feel like parsing the entire method to figure out how or why you're applying sales tax. For instance you can assume that people know that sales tax is different across states, but if you don't charge sales tax in North Carolina as a business decision or due to a legal loophole, put that in the docblock.

[SaranacLake]

3 minutes ago, maxxd said:

First off, tip #3 is something everyone who codes should live by. To add to it, don't string together a million conditionals - I'm currently dealing with a code base that has a 180-line if-elseif-else tree and it makes me wanna barf. For what is supposed to be a conditional logic operation, this is shockingly devoid of both condition and logic.

Not sure if I follow completely what you mean - as in how many nested IF-THEN-ELSE's?

Although, I think my IF-THEN-ELSE's would make you turn to violence?!  ☺️

I have IF-THEN-ELSE's that cover the majority of some of my scripts, and so they can be well over 1,000 lines of code.  (Surprisingly, I have become very adept at reading them, but I'm sure there is an easier way!)

 

3 minutes ago, maxxd said:

As far as length, methods should do one thing, and objects should handle one situation. What you've got now is what's called a god object in OOP, and it's - as you're finding - difficult to reason about and maintain.

Not so much a "God class", but rather a particular "use-case".  trouble is, the start-to-finish path of a use-case is often well over 1,00 lines of code.  Most of my scripts average 800-1,500 lines of code, and I have over 50 of them.

 

3 minutes ago, maxxd said:

On the other hand, the issue you can run into with one function per file is that it can become a mess of code relationship that's hard to follow if you're not extremely diligent.

True.  (If you want to want to know what's involved with say "checking out", it's all in one script minus a few functions.  So while scrolling can be a bitch on my laptop, I never have to go on an Easter egg hunt across 50 files to follow the flow of things.)

 

3 minutes ago, maxxd said:

It also absolutely requires a full test suite because chances are you'll be including the same file in multiple places, and this can lead to hard to find bugs when you forget you use a function somewhere in a different part of the code.

That is one of many things that scares me about OOP.

 

3 minutes ago, maxxd said:

And finally, just to beat the horse some more, format everything appropriately. I've spent untold hours at work trying to parse SQL queries because the original author wrote them out in one long line instead of using line breaks and indenting to make it make sense.

I probably average one line f comments for one line f code, and my code is some of the prettiest you'll ever see, so preaching to the chior on that one.  But still good to be said!

 

 

3 minutes ago, maxxd said:

Obviously PHP isn't Python or Ruby in that indentation doesn't actually affect the code, but there's a reason the authors of some programming languages chose to use indentation instead of brackets or curly braces - it makes the code easier to read, and illuminates the intention of the functionality.

Agreed.

 

 

3 minutes ago, maxxd said:

Wait - I lied; one more thing (it's late but I'm not sleepy and my mind is going, sorry). Absolutely use descriptive and appropriate method and variable names, but document as well. Just because your method is called `applySalesTax` doesn't mean I feel like parsing the entire method to figure out how or why you're applying sales tax. For instance you can assume that people know that sales tax is different across states, but if you don't charge sales tax in North Carolina as a business decision or due to a legal loophole, put that in the docblock.

As mentioned, probably 40-50% of my code is comments, so I get you.  (Not to say YOU would understand everything, but it is enough so that I have been able to read and understand 90% of my code from 8-10 years ago!  (And the few places I couldn't understand things - mainly some big ass queries - i should have done a better job documenting the business logic?!)

 

 

[kicken]

55 minutes ago, SaranacLake said:

Not sure if I follow completely what you mean - as in how many nested IF-THEN-ELSE's?

Rule #3 is about avoiding something like this:

function blah($list){
	foreach ($list as $a){
		if ($a['flag']){
			foreach ($a['list'] as $b){
				if ($b['flag']){
					//something
				} else if ($b['flag2']){
					foreach ($b['list'] as $c){
						if ($c['x']){
							//something
						} else {
							//other thing
						}
					}
				}
			}
		}
	}
}

It's hard to read, particularly when the //something's are longer than one line and it wastes horizontal space.  It'd be better to pull chunks out into other functions, for example:

function blah($list){
	foreach ($list as $a){
		if ($a['flag']){
			processFlagList($a);
		}
	}
}

function processFlaglist($list){
	foreach ($list as $b){
		if ($b['flag']){
			//something
		} else if ($b['flag2']){
			processFlag2($b);
		}
	}
}

function processFlag2List($list){
	foreach ($b['list'] as $c){
		if ($c['x']){
			//something
		} else {
			//other thing
		}
	}
}

Much easier to follow, especially if you only want a high-level overview of the code and don't need the details.  This can be valuable in some big if/else chain like described above. 

Imagine you were just quickly trying to get an idea of what some function does and you see this:

function blah($list){
	foreach ($list as $a){
		if ($a['flag1']){
			//something 20 lines long
		} else if ($a['flag2']){
			//something else 20 lines long
		} else if ($a['flag3']){
			//yet another thing 30 lines long
		} else {
			//and finally the last thing 8 lines long
		}
	}
}

Remember, imagine all those //something's are big blocks of code that prevent you from seeing the whole tree like you can here.  That'd be a whole lot of code to read through, figure out what it does and keep track of it all.  What if instead you saw this:

function blah($list){
	foreach ($list as $a){
		if ($a['flag1']){
			sendToProcessor($a);
		} else if ($a['flag2']){
			markCompleted($a);
		} else if ($a['flag3']){
			notifyTheCEO($a);
		} else {
			setEverythingOnFire();
		}
	}
}

Much easier and the names of the function let you know what's happening without having to dig deeper.  If you decide you need to know exactly how the code sets everything on fire you can easily* go find out, but if you don't care then that code isn't cluttering up your view. 

 

* With a good IDE going and finding out what a function does is easy.  In PHPStorm for example I could just CTRL+Click on (or CTRL+B with the text cursor in) the function name and it'll take me to it.  Without a good IDE it might be harder.

 

[SaranacLake]

11 minutes ago, kicken said:

Rule #3 is about avoiding something like this:

 

I just didn't know if this comment...

I'm currently dealing with a code base that has a 180-line if-elseif-else tree and it makes me wanna barf. 

meant an IF-THEN-ELSE with 180 lines in between the THEN and ELSE, or if @maxxd meant 180 IF-THEN-ELSEIF-ELSEIF-ELSEIF...

 

Either way, I get what both of you are saying, and thank you for the code examples.

 

Yes, that is one thing I suffer with in my code base - lots of scrolling because I do have all of the detail in one place.

Although, my solution to that is that I use comments like...

	// ****************************
	// DO SOMETHING (1)
	// ****************************
	code here....
	 
	// ****************************
	// DO SOMETHING (2)
	// ****************************
	more code here....
	

 

Not justifying things, but I am saying that looking for my "section heading" - plus anything that is grey in netBeans - makes it pretty easy to scroll and find what I need.

Every script also have a table of Contents...

	Do one thing (1)
	Do another (2)
	:
	:
	Do even more (30)
	

 

So I am pretty adept searching (or scrolling for "Do this (27)" to get quickly to where I need.

It's sorta like any disability....  You learn to over compensate in other areas and you end up surviving.

But having two eyes, two ears, two hands, etc would be nice moving forward!  😉