Won't fix Redundant code in XenForo_Model_UserGroup::addUserGroupChange


Well-known member
public function addUserGroupChange($userId, $key, $addGroups)
    if (is_array($addGroups))
        $addGroups = implode(',', $addGroups);
    if (!$addGroups)
        return true;
    $oldGroups = $this->getUserGroupChangesForUser($userId);
    $newGroups = $oldGroups;
    if (isset($newGroups[$key]) && !$addGroups) // L1237
        // already exists and we're removing the groups, so we can just remove the record
        return $this->removeUserGroupChange($userId, $key);
As you can see, if $addGroups is false the function will return true, however L1237 checks for $addGroups to see whether it's false again.
While you're right, we're talking about a single boolean check so there's no harm in it.
Top Bottom