Here we go again...

[CodingJungle]

it's been awhile @bfarber, and i don't want you thinking i've forgotten about you, so i went out of my way to get you something!

 

[bfarber]

Thanks, I've submitted a patch for review but I'll push off a fix for 4.5 given how low level the code is and how low priority and rare the issue itself is.

[TSP]

1 hour ago, bfarber said:

Thanks, I've submitted a patch for review but I'll push off a fix for 4.5 given how low level the code is and how low priority and rare the issue itself is.

While we are looking at this code. Why don't you just use isset instead of in_array? Seems to me it would be lighter in terms of performance. 

Simply: if (isset($this->words[$k]))
instead of: if (\in_array($k, array_keys($this->words)))

Or am I missing some reason you use this approach, which to me seems like more overhead?

I found a blog post on this: http://maettig.com/1397246220

Quote

The result really looks how you think it should look. Searching for a key in a hash table needs constant time, no matter how big the array is. That's exactly why we have hash tables, right? On the other hand, searching for a value in an array needs dramatically more time even on relatively small arrays with only 100 elements. For an array with 10000 elements in_array is more than 100 times slower.

So never ever do in_array( $key, array_keys( $array ) ), that's just stupid.

And some stackoverflow answers https://stackoverflow.com/questions/13483219/what-is-faster-in-array-or-isset

I haven't looked any closer into this code, than just watching the video, but if $this->words contains all the language strings, then I would expect some performance improvement by changing to use isset or array_key_exists (if you need to account for that the value could be null). 

Edited October 21, 2019 by TSP

[newbie LAC]

15 hours ago, TSP said:

Why don't you just use isset instead of in_array?

They use isset for 2nd part (else statement)

		if ( \is_array( $key ) )
		{
			foreach( $key as $k )
			{
				if ( \in_array( $k, array_keys( $this->words ) ) )
				{
					$return[ $k ] = $this->words[ $k ];
				}

				..........
			}

			..........
		}
		else
		{
			if ( isset( $this->words[ $key ] ) )
			{
				return $this->words[ $key ];
			}

			..........
		}

If I’m not mistaken there was a topic (or bug report) about this.

In the previous versions the code looks like

		if ( is_array( $key ) )
		{
			foreach( $key as $k )
			{
				if ( in_array( $k, array_keys( $this->words ) ) )
				{
					$return[ $k ] = $this->words[ $k ];
				}
				..........
			}
			
			..........
		}
		else
		{
			if ( in_array( $key, array_keys( $this->words ) ) )
			{
				return $this->words[ $key ];
			}

			..........
		}

So IPS changed ELSE part

[CodingJungle]

21 hours ago, bfarber said:

Thanks, I've submitted a patch for review but I'll push off a fix for 4.5 given how low level the code is and how low priority and rare the issue itself is.

at least you didn't say it was a "niche" issue 🙂 or i would've  been forced to break out the 'momo' again, but thanks, its all i was asking for 🙂, to make you guys aware of it.

21 hours ago, TSP said:

While we are looking at this code. Why don't you just use isset instead of in_array? Seems to me it would be lighter in terms of performance. 

Simply: if (isset($this->words[$k]))
instead of: if (\in_array($k, array_keys($this->words)))

Or am I missing some reason you use this approach, which to me seems like more overhead?

I found a blog post on this: http://maettig.com/1397246220

And some stackoverflow answers https://stackoverflow.com/questions/13483219/what-is-faster-in-array-or-isset

I haven't looked any closer into this code, than just watching the video, but if $this->words contains all the language strings, then I would expect some performance improvement by changing to use isset or array_key_exists (if you need to account for that the value could be null). 

i wondered this myself, why they didn't use array_key_exists, i didn't mention it in the video how i would've done it differently. from a performance POV i am not sure which would be faster, but i would imagine it would be one of those premature optimizations we were talking about the other day in slack, cause Lang::get() isn't used very often, not as much as addToStack is. 

 

Edited October 22, 2019 by CodingJungle

[CodingJungle]

i'm still hitting this problem, now with follow...can't you guys just do one of your ninja patches for this? :)

[bfarber]

I can't even recall what this issue is because the video is 15 minutes and I cba to re-watch it again since I've noted the issue is fixed in 4.5.

 

[CodingJungle]

2 minutes ago, bfarber said:

I can't even recall what this issue is because the video is 15 minutes and I cba to re-watch it again since I've noted the issue is fixed in 4.5.

 

Whoops\Exception\ErrorException thrown with message "Undefined index: 6c047061df4701e5d56a5c86b15d3d17_desc"

Stacktrace:
#7 Whoops\Exception\ErrorException in /home/michael/public_html/dev/system/Lang/Lang.php:656
#6 Whoops\Run:handleError in /home/michael/public_html/dev/system/Lang/Lang.php:656
#5 IPS\_Lang:get in /home/michael/public_html/dev/system/Helpers/Form/Form.php:463
#4 IPS\Helpers\_Form:customTemplate in /home/michael/public_html/dev/applications/core/modules/front/system/notifications.php:156
#3 IPS\core\modules\front\system\_notifications:options in /home/michael/public_html/dev/system/Dispatcher/Controller.php:85
#2 IPS\Dispatcher\_Controller:execute in /home/michael/public_html/dev/system/Dispatcher/Dispatcher.php:152
#1 IPS\_Dispatcher:run in /home/michael/public_html/dev/init.php:821
#0 IPS\toolbox_hook_Dispatcher:run in /home/michael/public_html/dev/index.php:13

it is basically a type coercion, using the third parameter on in_array forces an identical check instead of just a equals check. it is just aggravating after 6 months the bug persist.

[CodingJungle]

Whoops\Exception\ErrorException thrown with message "Method IPS\Helpers\Form::__toString() must not throw an exception, caught Whoops\Exception\ErrorException: Undefined index: 6dbef3c91911fb0a0a055257a3ccd7d8_desc"

Stacktrace:
#2 Whoops\Exception\ErrorException in /home/michael/public_html/dev/system/Theme/Theme.php:4316
#1 Whoops\Run:handleError in /home/michael/public_html/dev/dev/Whoops/Run.php:433
#0 Whoops\Run:handleShutdown in [internal]:0

i'm also hitting this error when editing member profiles in the acp. 

[Martin A.]

Whoops\Exception\ErrorException thrown with message "Method IPS\Helpers\Form::__toString() must not throw an exception, caught Whoops\Exception\ErrorException: Undefined index: developer_build_type_desc"

Stacktrace:
#2 Whoops\Exception\ErrorException in /tmp/theme_core_admin_global_globalTemplateWqIz0B:0
#1 Whoops\Run:handleError in /home/sites/dev/www/ips44/dev/Whoops/Run.php:433
#0 Whoops\Run:handleShutdown in [internal]:0

I got this today trying to get to the app download page.

Hadn't it been for @CodingJungle who made me aware of this issue, I could have wasted hours rolling back to older PHP versions, reinstalling, etc.

The patch for this must be pretty simple, as all I had to do was add ", TRUE" to the code, and that makes me wonder why you haven't included this is any of the 5 releases (not including silent patches) since this was marked as solved.