244
edits
Update:Archive/1.0/Developers: Difference between revisions
Update:Archive/1.0/Developers (view source)
Revision as of 15:23, 19 February 2005
, 19 February 2005→/update/VersionCheck.php
| Line 523: | Line 523: | ||
* [52] This is a pointless wrapper. | * [52] This is a pointless wrapper. | ||
* [63,113,124,138,144,265] bail() is questionable; shouldn't we let things die quietly? | * [63,113,124,138,144,265] bail() is questionable; shouldn't we let things die quietly? | ||
(alanjstr says: can we make bail() output valid XML, maybe outputting the correct format of input?) | |||
* [59+] Functions defined during script - this should be separated; the logic behind the script is hard to determine because function definitions interrupt it | * [59+] Functions defined during script - this should be separated; the logic behind the script is hard to determine because function definitions interrupt it | ||
* [127-132] This seemingly arbitrary variable renaming makes it hard to follow where data originates. | * [127-132] This seemingly arbitrary variable renaming makes it hard to follow where data originates. | ||
| Line 530: | Line 531: | ||
** > 0 is not sufficient; it should be !== FALSE since strpos can return 0 as a valid return | ** > 0 is not sufficient; it should be !== FALSE since strpos can return 0 as a valid return | ||
** Code comments indicate 1-6, checks provide IDs 0,2,3,4,5,6 | ** Code comments indicate 1-6, checks provide IDs 0,2,3,4,5,6 | ||
** (alanjstr says: replace this with names, not ints) | |||
** Use logic to create short-circuit; right now it's all if's so the checks will happen even after a match is found and $osid is set -- logically, it's possible for $osid to be set twice | ** Use logic to create short-circuit; right now it's all if's so the checks will happen even after a match is found and $osid is set -- logically, it's possible for $osid to be set twice | ||
* [234] Query in a loop. Remove this. This could be replaced with an IN( explode(','$reqItemGuid) ) instead of $reqItemGuid[$i] in the query (assuming some checks on $reqItemGuid before the explode()), reducing the number of queries from count($reqItemGuid) to 1 since $osid and $reqTargetAppGuid are the same. | * [234] Query in a loop. Remove this. This could be replaced with an IN( explode(','$reqItemGuid) ) instead of $reqItemGuid[$i] in the query (assuming some checks on $reqItemGuid before the explode()), reducing the number of queries from count($reqItemGuid) to 1 since $osid and $reqTargetAppGuid are the same. | ||
** (alanjstr says: good catch. This is new code, so it only handles 1 at a time right now) | |||
* [253] Is there any way to avoid this OR ? | * [253] Is there any way to avoid this OR ? | ||