Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core] Add getKey function #3275

Merged
merged 2 commits into from
Mar 2, 2023
Merged

Conversation

Bockiii
Copy link
Contributor

@Bockiii Bockiii commented Mar 2, 2023

This will add a general "getKey" function to the BridgeAbstract that can retrieve key names from multidimensional arrays.

Problem:
There is currently no prepared way to retrieve the key name of a selected value for use in the output.

Example:
You have an array that contains countries and country codes ( eg 'United States' => 'us') and want to use the key name 'United States' in your feed output (a common use case for feed names). There is no onboard way to retrieve the key name and you will have to write your own code to match your exact use case. It gets even worse if the array is nested ( 'North America' => [ 'United States' => 'us'] ).

This code solves this. To show, I removed my own, exact matching use case from the JustWatch bridge and added the usage of the general function. It works with strings and integers and all of my personal tests have succeeded.

If you want this, I can also add documentation to the docs part before you merge.

@github-actions
Copy link

github-actions bot commented Mar 2, 2023

Pull request artifacts

file last change
JustWatchBridge-current-context1 2023-03-02, 02:23:45
JustWatchBridge-pr-context1 2023-03-02, 02:23:45

@dvikan
Copy link
Contributor

dvikan commented Mar 2, 2023

This is for lists only? Maybe better name is getKeyFromListValue().

Can also do:

$language = 'de-DE';
$languageValues = $this->getParameters()[0]['language']['values'];
$languageValuesFlipped = array_flip($languageValues);
// 'German'
$languageKey = $languageValuesFlipped[$language] ?? null;

Would be nice with a unit test.

@dvikan dvikan merged commit f3f98a1 into RSS-Bridge:master Mar 2, 2023
@Bockiii
Copy link
Contributor Author

Bockiii commented Mar 2, 2023

Not sure if you were planning on just merging this without the proposed changes :D

Sure, your name makes sense as well. Yes, it should also do the example that you wrote.

I will do another PR where I go through the current bridges and replace their "custom" versions with this central function. Also will add the docu information on how to use it.

As for the unit test: No idea how to do that, thats something you can do after the next PR :)

@Bockiii
Copy link
Contributor Author

Bockiii commented Mar 2, 2023

No, wait. It cant.

Its also not really "get by value", more a "get by array name" as the value is implied by providing the array name.

Thats why you provide it with the argument "language" and not "de-DE". From the array selection "language", it will pull the selected value from the inputs (so it will find that you selected "de-DE" for "language") and then traverse the (possibly multidimensional) array "language" to find the key name for the value "de-DE".

Its the same usage as getInput, since you also would use getInput('language') to retrieve the value "de-DE".

@Bockiii
Copy link
Contributor Author

Bockiii commented Mar 2, 2023

One of the problems (and why we need to do this in the first place) is that the static::PARAMETERS does not contain the key, only the value. For your example, it would look like 'language' => [ 0 => 'de-DE' ]. It would drop the "german". If we fix that in the setDatas function, we might be able to do it easier (by invoking static::PARAMETERS with an array_flip.) but I wasnt smart enough to do that in the setDatas function without possibly breaking everything.

@dvikan
Copy link
Contributor

dvikan commented Mar 2, 2023

I'm just gonna merge your PRs without spending more energy in this.

@dvikan
Copy link
Contributor

dvikan commented Jul 2, 2023

no big deal but causes noise in logs and will probably exception in later php versions.

@Bockiii

[2023-07-02 04:48:07] rssbridge.WARNING Array to string conversion at lib/BridgeAbstract.php line 319

code:

foreach (static::PARAMETERS[$context][$input]['values'] as $first_level_key => $first_level_value) {
            // todo: this cast emits error if it's an array
            $valueString = (string) $first_level_value;
            if ($needle === $valueString) {
                return $first_level_key;
            } elseif (is_array($first_level_value)) {
                foreach ($first_level_value as $second_level_key => $second_level_value) {
                    if ($needle === (string)$second_level_value) {
                        return $second_level_key;
                    }
                }
            }
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants