Inline Functions?

mongoose00318 · May 8, 2020

I've seen this recently when looking at some PHP code and it feels very familiar because it's done in Javascript a lot. But, I don't have a good understanding of it even then...also, I'm not sure "inline functions" is the actual names of these...here is a sample...

$cities = ['Berlin', 'KYIV', 'Amsterdam', 'Riga'];
$aliases = array_map('strtolower', $cities);
 
print_r($aliases); // ['berlin', 'kyiv, 'amsterdam', 'riga']
 
$numbers = [1, -2, 3, -4, 5];
$squares = array_map(function($number) {
    return $number ** 2;
}, $numbers);
 
print_r($squares);  // [1, 4, 9, 16, 25]

So where it says (array_map(function($number) {....

I'm confused about this part...normally when I write a function like...

function($some_var) {
    return $some_var;
}

$some_var is acting as an input to the function...with the function sample above is $number acting as an output? $number has no value before the declaration of the function right? Hopefully I've worded this all correctly...

mongoose00318 · May 8, 2020

Also, what does the &$value do vs just $value in the below example?

array_walk( $fruits, function( &$value, $key ) {
	$value = "$key is ".strtoupper($value);
});

requinix · May 8, 2020

25 minutes ago, mongoose00318 said:

I'm not sure "inline functions" is the actual names of these...

Just about everyone will recognize what you're describing with that name, but they're officially called "closures" or "anonymous functions".

28 minutes ago, mongoose00318 said:

So where it says (array_map(function($number) {....

It's a function like any other. It takes an argument of $number and returns something. There's nothing particularly special about how it works.

20 minutes ago, mongoose00318 said:

Also, what does the &$value do vs just $value in the below example?

The & indicates a reference. array_walk has a special behavior where you can modify the value in the array - but instead of returning the new value, you mark the first argument in your function as being by-reference and modify it directly.

mongoose00318 · May 8, 2020

1 minute ago, requinix said:

It's a function like any other. It takes an argument of $number and returns something. There's nothing particularly special about how it works.

So $number is essentially being returned as an argument for the array_map() function?

requinix · May 8, 2020

Your terminology is incorrect (an "argument" is something passed to a function when it's called) but yes: the function returns a new value which array_map uses in the new array it's creating.

mongoose00318 · May 8, 2020

Okay very cool. Thanks a lot @requinix

mongoose00318 · May 8, 2020

I'll post some code later that I wrote yesterday building an array which is then passed as JSON to generate a Gannt chart on the fly. I'm proud of it for now...until you guys review it and tell me how repetitive my code is haha. It's cool learning to be more efficient with my code and really cool learning how to better build and interact with arrays. Back when I was still developing full time before I stepped away from it for several years; arrays was always something I had trouble with. I feel like I'm learning a lot about them now and it's making my life a lot easier.

mongoose00318 · May 8, 2020

@requinix Here is that code I mentioned earlier...

//fetch production history data
if( isset ( $action ) && $action == 'get_production_history' ) {
	$order_id = $_GET['order_id'];
	$production_history = [];
	$last_recorded = 0;
	$dept_codes = [5,6,7,8,10,11,12];
	
	foreach ( $dept_codes as $d ) {
		$production_history[$d] = [];
	}
	
	//echo '<pre>';
	
	//loop through depts
	foreach ( $dept_codes as $d ) {
		
		//loop through returned db rows
		foreach ( get_production_history( $order_id, $pdo ) as $row ) {
			
			if( $row['dept_code'] == $d ) {
				//set start time
				if ( !in_array ( $row[ 'id' ], $production_history[ $d ]) && $row[ 'status_id' ] === 1 ) {

					$production_history[$d][ $row['id'] ] = array(
						'submit_time' => $row[ 'submit_time' ],
						'finish_time' => '',
					);

					//record id
					$last_recorded = $row['id'];

				//set finished time
				} elseif ( $row[ 'status_id' ] === 3 ) {
					$production_history[$d][ $last_recorded ]['finish_time'] = $row[ 'submit_time' ];
				}
				
			}
			
		}
		
	}
	
	//find records without a finish time and unset them
	foreach ( $production_history as $dept => $value ) 
		foreach ($value as $k => $v) 
			if (empty($v['finish_time'])) 
				unset($production_history[$dept][$k]);
	
	//find departments without records and unset them
	foreach ( $production_history as $dept => $value )
		if (empty($value)) 
			unset($production_history[$dept]);
	
	$json = [];
	
	$dept_arr_count = 0;
	//sort by dept
	foreach ( $production_history as $dept => $value ) {
		
		//get length of dept array
		$dept_arr_size = count($production_history[$dept]);
		
		//if on first entry for dept print parent
		if ( $dept_arr_count == 0 ) {
			
			//generate parent JSON entry
			$json[] = array(
				'pID' => $dept,
				'pName' => get_dept_name( $dept, $pdo ),
				'pStart' => '',
				'pEnd' => '',
				'pClass' => 'ggroupblack',
				'pLink' => '',
				'pMile' => 0,
				'pGroup' => 2,
				'pParent' => 0, //need to set this for those that are children
				'pOpen' => 1,
				'pDepend' => '',
				'pCaption' => '',
				'pNotes' => '',
			);
			
		} 
		
		$submit_time = (isset($production_history[$dept][$k]['submit_time'])) ? $production_history[$dept][$k]['submit_time'] : '';
		$finish_time = (isset($production_history[$dept][$k]['finish_time'])) ? $production_history[$dept][$k]['finish_time'] : '';
		//print children
		foreach ($value as $k => $v) {
			$json[] = array(
				'pID' => $dept .''. $dept_arr_count+1,
				'pName' => '',
				'pStart' => $production_history[$dept][$k]['submit_time'],
				'pEnd' => $production_history[$dept][$k]['finish_time'],
				'pClass' => 'ggroupblack',
				'pLink' => '',
				'pMile' => 0,
				'pGroup' => 0,
				'pParent' => $dept, //need to set this for those that are children
				'pOpen' => 1,
				'pDepend' => '',
				'pCaption' => '',
				'pNotes' => '',
			);
			$dept_arr_count++;
		}
		
		//reached end of dept array
		if ( $dept_arr_size == $dept_arr_count ) {
			$dept_arr_count = 0;
		}
	}
	
	header('Content-type: text/javascript');
	print(json_encode($json, JSON_PRETTY_PRINT));
}

I was pretty proud of it haha. Here is what it generates...

https://imgur.com/JW9Npz2

requinix · May 8, 2020

if( isset ( $action ) && $action == 'get_production_history' ) {

Where is $action being defined? If it's coming from $_GET/POST then that is where the isset() should be. Because the point of using it is to make sure it really does exist (or else PHP will complain that it does not). But once you've assigned something to a variable, it definitely does exist (even if the thing you were assigning to it did not), so the isset() is probably unnecessary.

foreach ( $dept_codes as $d ) {
    //loop through returned db rows
    foreach ( get_production_history( $order_id, $pdo ) as $row ) {

get_production_history() does a database query, right? Given that you aren't passing it any department information, won't this run the exact same query over and over again? If that is the case then you should switch these two foreachs around so that the database one only happens once while the department one (which just goes over a small array) happens more than once.

if( $row['dept_code'] == $d ) {

If you only care about production history data matching a specific department then running a query for everything is highly wasteful. This problem partly goes away if you do the thing I said above, but better would be to find a way to run a modified query for production data that only includes results for the departments you want.

if ( !in_array ( $row[ 'id' ], $production_history[ $d ]) && $row[ 'status_id' ] === 1 ) {

Does this mean that the query will somehow be returning multiple rows with the same ID and dept_code for status_id=1? That doesn't sound good.

} elseif ( $row[ 'status_id' ] === 3 ) {

How do you know that this $row corresponds to the earlier $last_recorded row? Wouldn't it just be easier to forget $last_recorded and use $row's ID in here?

foreach ( $dept_codes as $d ) {...
foreach ( get_production_history( $order_id, $pdo ) as $row ) {... // assuming you rearrange these two loops like I said
    foreach ( $dept_codes as $d ) {....
foreach ( $production_history as $dept => $value )...
    foreach ($value as $k => $v)...
foreach ( $production_history as $dept => $value )...
foreach ( $production_history as $dept => $value ) {...
    foreach ($value as $k => $v) {...

That's a lot of loops over the same data. You should be able to combine some of these.
Tip: use a continue to skip the rest of the code in a looped block.

header('Content-type: text/javascript');

text/javascript means Javascript, and while you are outputting valid Javascript, that's not what you're trying to do. JSON is "application/json".

kicken · May 8, 2020

6 hours ago, mongoose00318 said:

until you guys review it and tell me how repetitive my code is haha.

Ok

foreach ( $dept_codes as $d ) {
	$production_history[$d] = [];
}

This can be replaced with array_fill_keys

$production_history = array_fill_keys($dept_codes, []);

Next up,

foreach ( $dept_codes as $d ) {

	//loop through returned db rows
	foreach ( get_production_history( $order_id, $pdo ) as $row ) {

		if( $row['dept_code'] == $d ) {

This ends up querying your database count($dept_codes) times when you only really need to do so once as far as I can tell. For each $dept_codes loop you're querying the database for all your results, then just filtering them out to only the ones matching the current department. Instead of doing that, just query your database once and process the results according to the department in the current row.

foreach (get_production_history($order_id, $pdo) as $row){
	$d = $row['dept_code'];
	//set start time
	if ( !in_array ( $row[ 'id' ], $production_history[ $d ]) && $row[ 'status_id' ] === 1 ) {

		$production_history[$d][ $row['id'] ] = array(
			'submit_time' => $row[ 'submit_time' ],
			'finish_time' => '',
		);

		//record id
		$last_recorded = $row['id'];

	//set finished time
	} elseif ( $row[ 'status_id' ] === 3 ) {
		$production_history[$d][ $last_recorded ]['finish_time'] = $row[ 'submit_time' ];
	}

}

Since your thread was prompted by anonymous functions, why not use them?

//find records without a finish time and unset them
foreach ( $production_history as $dept => $value )
	foreach ($value as $k => $v)
		if (empty($v['finish_time']))
			unset($production_history[$dept][$k]);

//find departments without records and unset them
foreach ( $production_history as $dept => $value )
	if (empty($value))
		unset($production_history[$dept]);

Could be replaced with a mix of array_filter and array_walk:

array_walk($production_history, function(&$entryList){
	$entryList = array_filter($entryList, function($value){
		return !empty($value['finish_time']);
	});
});
$production_history = array_filter($production_history);

First array_walk loops over the $production_history array like your foreach loop, each value is passed to the function as $entryList (using a reference so we can modify it). Then the function uses array_filter to keep only entries which have a finish_time defined.

//if on first entry for dept print parent
if ( $dept_arr_count == 0 ) {

	//generate parent JSON entry
	$json[] = array(
		'pID' => $dept,
		'pName' => get_dept_name( $dept, $pdo ),
		'pStart' => '',
		'pEnd' => '',
		'pClass' => 'ggroupblack',
		'pLink' => '',
		'pMile' => 0,
		'pGroup' => 2,
		'pParent' => 0, //need to set this for those that are children
		'pOpen' => 1,
		'pDepend' => '',
		'pCaption' => '',
		'pNotes' => '',
	);
}

The if statement check here is unnecessary the way your data is setup. $production_history is a unique array of departments. As such, this part of the loop will only run once per department and there's no need to try and track if you're on a new department.

$submit_time = (isset($production_history[$dept][$k]['submit_time'])) ? $production_history[$dept][$k]['submit_time'] : '';
$finish_time = (isset($production_history[$dept][$k]['finish_time'])) ? $production_history[$dept][$k]['finish_time'] : '';

$k is meaningless here. It's value is going to be whatever the last entry of the //find records without a finish time and unset them loop is (or undefined in my replacement version) You're not using these variables anywhere anyway so the entire lines could be removed. I'm not sure what you were attempting with these lines.

$dept .''. $dept_arr_count+1

You might have an operator precedence issue here, it's unclear what your goal is.

Concatenation and addition have the same precedence and are processed left to right so that expression is ($dept.$dept_arr_count)+1. You combine $dept and $dept_arr_count into a single number, then add one to it.

Your spacing however, to me, suggests you intend to add one to $dept_arr_count first, then combine the two numbers. Achieving that requires a set of parenthesis. $dept . ($dept_arr_count+1)

In either case, the empty string is unnecessary and can be removed.

$production_history[$dept][$k]['submit_time']
$production_history[$dept][$k]['finish_time']

$production_history[$dept][$k] is the same as $v, so you can just use $v['submit_time'] and $v['finish_time'] instead.

So, with all those suggestions applied, the new code would look something like this, assuming I didn't goof up somewhere:

//fetch production history data
if( isset ( $action ) && $action == 'get_production_history' ) {
    $order_id = $_GET['order_id'];
    $production_history = [];
    $last_recorded = 0;
    $dept_codes = [5,6,7,8,10,11,12];
    
    $production_history = array_fill_keys($dept_codes, []);
    
    //loop through returned db rows
    foreach (get_production_history($order_id, $pdo) as $row){
        $d = $row['dept_code'];
        //set start time
        if ( !in_array ( $row[ 'id' ], $production_history[ $d ]) && $row[ 'status_id' ] === 1 ) {

            $production_history[$d][ $row['id'] ] = array(
                'submit_time' => $row[ 'submit_time' ],
                'finish_time' => '',
            );

            //record id
            $last_recorded = $row['id'];

        //set finished time
        } elseif ( $row[ 'status_id' ] === 3 ) {
            $production_history[$d][ $last_recorded ]['finish_time'] = $row[ 'submit_time' ];
        }
    }

    
    array_walk($production_history, function(&$entryList){
        $entryList = array_filter($entryList, function($value){
            return !empty($value['finish_time']);
        });
    });
    $production_history = array_filter($production_history);
    
    $json = [];
    foreach ( $production_history as $dept => $value ) {
        //generate parent JSON entry
        $json[] = array(
            'pID' => $dept,
            'pName' => get_dept_name( $dept, $pdo ),
            'pStart' => '',
            'pEnd' => '',
            'pClass' => 'ggroupblack',
            'pLink' => '',
            'pMile' => 0,
            'pGroup' => 2,
            'pParent' => 0, //need to set this for those that are children
            'pOpen' => 1,
            'pDepend' => '',
            'pCaption' => '',
            'pNotes' => '',
        );

        //print children
        $dept_arr_count = 0;
        foreach ($value as $k => $v) {
            $json[] = array(
                'pID' => $dept .''. $dept_arr_count+1,
                'pName' => '',
                'pStart' => $v['submit_time'],
                'pEnd' => $v['finish_time'],
                'pClass' => 'ggroupblack',
                'pLink' => '',
                'pMile' => 0,
                'pGroup' => 0,
                'pParent' => $dept, //need to set this for those that are children
                'pOpen' => 1,
                'pDepend' => '',
                'pCaption' => '',
                'pNotes' => '',
            );
            $dept_arr_count++;
        }
    }
    
    header('Content-type: text/javascript');
    print(json_encode($json, JSON_PRETTY_PRINT));
}

Sign In

Inline Functions?

Recommended Posts

mongoose00318

Link to comment

Share on other sites

mongoose00318

Link to comment

Share on other sites

requinix

Link to comment

Share on other sites

mongoose00318

Link to comment

Share on other sites

requinix

Link to comment

Share on other sites

mongoose00318

Link to comment

Share on other sites

mongoose00318

Link to comment

Share on other sites

mongoose00318

Link to comment

Share on other sites

requinix

Link to comment

Share on other sites

kicken

Link to comment

Share on other sites

Join the conversation

Browse

Activity

Important Information