-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[BadDragonBridge] Add new bridge #1082
Conversation
} | ||
} | ||
|
||
public function detectParameters($url) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the point of this function ?
Same for setParam
, that is only called in this function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
detectParameters
is for automatically selecting a bridge from a URL. See wiki here.
setParam
sets index $strFrom
(or $strTo
if it's set) in $outArr
to 'on' if $inArr[$param]
contains $strFrom
. It's for translating their shop filter URL parameters into something rss-bridge can use, for example:
Array
(
[type] => Array
(
[0] => ready_made
[1] => flop
)
[firmnessValues] => Array
(
[0] => medium
)
)
to:
Array
(
[ready_made] => on
[flop] => on
[med_firm] => on
)
Perhaps there's some nicer way to do that though, or at least a comment explaining setParam
could be added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, the function setParam
is hard to comprehend because the arguments (and the function name) don't add to clarity. It's generally good practice to name variables according to their purpose, not their type.
What do $inArr
, $outArr
, $param
, $strFrom
and $strTo
really represent?
From your explanation I understand that the function should be named transformQuery
(or similar). It also makes sense to include an example in the documentation for future reference.
Does this work for you?
/**
* Adds one element from a multi-value input query to a single-value
* output query, using the value of the input element as parameter
* name in the output query and setting the value to 'on'.
*
* The parameter name can optionally be changed with $outputParam
*
* Example
*
* parse_str('?type[]=ready_made&type[]=flop', $inputQuery);
* $outputQuery = array();
*
* setParam($inputQuery, $outputQuery, 'type', 'ready_made', 'ready');
* setParam($inputQuery, $outputQuery, 'type', 'flop');
*
* // $outputQuery === array(
* // 'ready' => 'on',
* // 'flop' => 'on'
* // );
*
*/
private function transformQuery($inputQuery, &$outputQuery, $inputParam, $inputValue, $outputParam = null) {
$outputParam = (is_null($outputParam)) ? $inputValue : $outputParam;
if(isset($inputQuery[$inputParam]) && in_array($value, $inputQuery[$inputParam])) {
$outputQuery[$outputParam] = 'on';
}
}
Makes more sense ! Thanks for taking in account all the comments and for creating this bridge ! |
* [BadDragonBridge] Add new bridge
New bridge for the Bad Dragon adult toy shop.