Многократное условие IF

Question

У меня есть утверждение if с несколькими условиями, которое я просто не могу понять, правильно

if(((ISSET($_SESSION['status'])) && ($_SESSION['username']=="qqqqq")) ||     
((ISSET($_SESSION['status'])) && ($_SESSION['company']=="wwwwww")) || 
((ISSET($_SESSION['status'])) && ($_SESSION['company']=="ffffffff")) || 
((ISSET($_SESSION['status'])) && ($_SESSION['company']=="ggggggg")) || 
((ISSET($_SESSION['status'])) && ($_SESSION['company']=="hhhhhhh")) || 
($_SESSION['LOGINTYPE']=="ADMIN")) {

Предполагается, что он будет возвращать true, если установлен статус, а также одно из названий этих компаний. Иначе, если logintype is admin, он также должен возвращать true независимо от компании или статуса

Answer 1

Как насчет этого:

if (
  (isset($_SESSION['status']) && $_SESSION['username']=="qqqqq")
  || (isset($_SESSION['status']) && in_array($_SESSION['company'], array('wwwwww', 'ffffffff', 'ggggggg', 'hhhhhhh')))
  || $_SESSION['LOGINTYPE']=="ADMIN"
)

Может помочь переписывание условия, чтобы сделать его немного более простым для чтения:

• Один возможный случай в строке:
  • установлен статус и имя пользователя ОК
  • установлен статус и компания ОК
  • admin
• Использование in_array вместо нескольких тестов в компании - Iдумаю, что это легче понять

Answer 2

Как минимум, переделайте его так, чтобы он читал то, что вы сказали, что он должен делать:

Предполагается, что он будет возвращать true, если установлен статус, а также одно из названий этих компаний. Иначе, если logintype is admin, он также должен возвращать true независимо от компании или статуса

function mayAccessResource(array $sessionState)
{
    return (hasStatus($sessionState) && isAllowedUserOrCompany($sessionState))
        || isAdmin($sessionState);
}

Затем добавьте методы для инкапсулирующих одиночных тестов:

function hasStatus($sessionState)
{
    return isset($sessionState['status']);
}

function isAllowedUserOrCompany($sessionState)
{
    return isAllowedUser($sessionState) || isAllowedCompany($sessionState);
}

function isAllowedUser($sessionState)
{
    return isset($sessionState['user']) 
        && $sessionState['user'] === 'blah';
}

function isAllowedCompany($sessionState)
{
    $allowedCompanies = array('foo', 'baz', 'baz');
    return isset($sessionState['company']) 
        && in_array($sessionState['company'], $allowedCompanies);
}

function isAdmin($sessionState)
{
    return isset($sessionState['LOGINTYPE'])
        && $sessionState['LOGINTYPE'] === 'admin';
}

Это все еще далеко от совершенства, поскольку он жестко кодирует слишком много информации, но, по крайней мере, теперь он гораздо более читабелен. Так как все они работают в состоянии сеанса, было бы намного красивее, если бы существовал объект SessionState. Еще лучше было бы разделить различные возможные состояния сеанса дальше и иметь класс Company, User и AccessControl.

Также обратите внимание на комментарии под вашим вопросом.

Answer 3

Другой вариант:

if (!$_SESSION['LOGINTYPE']=="ADMIN") {
    return false;
}

if (!ISSET($_SESSION['status']) {
    return false
}

if ($_SESSION['username']=="qqqqq") {
    return true;
}

// etc

Или использовать переключатель, начиная с третьего условного.

Еще лучше было бы сделать объектно-ориентированным:

if ($this->user->isAdmin()) || $this->user->belongsToCompany($this->allowedCompanies)

Это читается еще лучше благодаря созданию «языка более высокого уровня».

Примечание: речь идет не о том, чтобы набрать как можно меньше символов на как можно меньшем числе строк.

Answer 4

Некоторое форматирование сделало бы это намного проще для умственного анализа =) Плюс, вы, вероятно, могли бы упростить аспект компаний, создав множество разрешенных компаний. Тестирование по одному делает это, если () трудно поддерживать и читать - каждый раз, когда вы добавляете компанию, вы рискуете нарушить логику.

Кроме того, похоже, что если $_SESSION['LOGINTYPE'] == 'ADMIN', это переопределение, которое всегда должно быть истинным, в противном случае вам всегда нужно устанавливать $ _SESSION ['status'].

$companies = array(
    'wwww'=>true,
    'ffff'=>true,
    'gggg'=>true,
    'hhhh'=>true
);
if (
    $_SESSION['LOGINTYPE'] == 'ADMIN'
||  isset($_SESSION['status'])
    && (
        $_SESSION['username'] == 'qqqqq'
    ||  isset($companies[$_SESSION['company']])
    )
) {
    ...
}

Или, может быть, более четко:

$pass = false;
if ($_SESSION['LOGINTYPE'] == 'ADMIN') {
    $pass = true;
} else if (isset($_SESSION['status'])) {
    if ($_SESSION['username'] == 'qqqq') $pass = true;
    if (isset($companies, $_SESSION['company']) $pass = true;
}
if ($pass) {
    ...
}

Answer 5

Почему бы вам не использовать try-catch-blocks и установить для переменной значение true, если регистр становится истинным?

Гораздо понятнее читать и поддерживать.

try($foo) {
    catch x:
    $bar = true;

    default:
    $bar = false;
}