Можно ли улучшить эту функцию PHP?

Question

Ниже приведен код, над которым я работаю для меню навигации. Если вы находитесь на определенной странице, он добавит «текущий» класс css на соответствующую вкладку.

Мне любопытно, есть ли лучший способ сделать это в PHP, потому что действительно кажется, что для выполнения такой простой задачи требуется много кода? На моих страницах также будет загружена библиотека jquery. Было бы лучше установить вкладку с помощью jquery вместо PHP? Любые советы приветствуются

<?PHP

active_header('page identifier goes here'); //ie; 'home' or 'users.online'

function active_header($page_name)
{

    // arrays for header menu selector
    $header_home = array('home' => true);
    $header_users = array(
        'users.online' => true,
        'users.online.male' => true, 
        'users.online.female' => true, 
        'users.online.friends' => true, 
        'users.location' => true, 
        'users.featured' => true, 
        'users.new' => true, 
        'users.browse' => true, 
        'users.search' => true, 
        'users.staff' => true
    );
    $header_forum = array('forum' => true);
    $header_more = array(
        'widgets' => true, 
        'news' => true, 
        'promote' => true, 
        'development' => true, 
        'bookmarks' => true, 
        'about' => true
    );
    $header_money = array(
        'account.money' => true, 
        'account.store' => true, 
        'account.lottery' => true, 
        'users.top.money' => true
    );
    $header_account = array('account' => true);
    $header_mail = array(
        'mail.inbox' => true, 
        'mail.sentbox' => true, 
        'mail.trash' => true, 
        'bulletins.post' => true, 
        'bulletins.my' => true, 
        'bulletins' => true
    );

    // set variables if there array value exist
    if (isset($header_home[$page_name])){
        $current_home = 'current';
    }else if (isset($header_users[$page_name])){
        $current_users = 'current';
    }else if (isset($header_forum[$page_name])){
        $current_forum = 'current';
    }else if (isset($header_more[$page_name])){
        $current_more = 'current';
    }else if (isset($header_money[$page_name])){
        $current_money = 'current';
    }else if (isset($header_account[$page_name])){
        $current_account = 'current';
    }else if (isset($header_mail[$page_name])){
        $current_mail = 'current';
    }

    // show the links
    echo '<li class="' . (isset($current_home) ? $current_home : '') . '"><a href=""><em>Home</em></a></li>';
    echo '<li class="' . (isset($current_users) ? $current_users : '') . '"><a href=""><em>Users</em></a></li>';
    echo '<li class="' . (isset($current_forum) ? $current_forum : '') . '"><a href=""><em>Forum</em></a></li>';
    echo '<li class="' . (isset($current_more) ? $current_more : '') . '"><a href=""><em>More</em></a></li>';
    echo '<li class="' . (isset($current_money) ? $current_money : '') . '"><a href=""><em>Money</em></a></li>';
    echo '<li class="' . (isset($current_account) ? $current_account : '') . '"><a href=""><em>Account</em></a></li>';
    echo '<li class="' . (isset($current_mail) ? $current_mail : '') . '"><a href=""><em>Mail</em></a></li>';
}

?>

Answer 1

Два очень больших блока кода внизу могут быть значительно сокращены до простого цикла:

<?php

foreach (array('home', 'users', 'forum' /* ... */ ) as $item) {
  $ar = "header_$item";
  echo '<li class="', (isset($$ar[$page_name]) ? 'current' : '')
    , '"><a href=""><em>', ucword($item), '</em></a></li>';
}

?>

Answer 2

Вы должны стараться не печатать <li class=""> или что-то в этом роде; это выглядит грязно. Я переместил проверку того, должна ли эта страница выделяться в отдельную функцию на случай, если вы в конечном итоге измените макет $applicable_list.

<?php
function active_header($page) {
    $applicable_list = array(
        "home" => array("home"),
        "users" => array(
            "users.online", "users.online.male", "users.online.female", "users.online.friends", 
            "users.location", "users.featured", "users.new", "users.browse", "users.search", "users.staff"
        ), 
        "forum" => array("forum"),
        "more" => array("widgets", "news", "promote", "development", "bookmarks", "about"),
        "money" => array("account.money", "account.store", "account.lottery", "users.top.money"),
        "account" => array("account"),
        "mail" => array("mail.inbox", "mail.sentbox", "mail.trash", "bulletins.post", "bulletins.my", "bulletins")
    );
    $pages = array_keys($applicable_list);

    function is_active_page($page, $category, $category_pages_list) {
        return array_key_exists($category, $category_pages_list) && in_array($page, $category_pages_list[$category]);
    }
    foreach($pages as $key => $category) {
        printf('<li%s><a href="#"><em>%s</em></a></li>' . "\n", 
            (is_active_page($page, $category, $applicable_list) ? ' class="current"' : ''),
            ucwords($category)
        );
    }
}

?>

Answer 3

Можно также добавить мою судьбу. Вывод ограничен, чтобы соответствовать указанному в исходном вопросе.

<?PHP

active_header('page identifier goes here'); //ie; 'home' or 'users.online'

function active_header($page_name)
{
    // Unified array
    $headers = array(
        'Home' => array('home' => true),
        'Users' => array(
            'users.online' => true,
            'users.online.male' => true, 
            'users.online.female' => true, 
            'users.online.friends' => true, 
            'users.location' => true, 
            'users.featured' => true, 
            'users.new' => true, 
            'users.browse' => true, 
            'users.search' => true, 
            'users.staff' => true
        ),
        'Forum' => array('forum' => true),
        'More' => array(
            'widgets' => true, 
            'news' => true, 
            'promote' => true, 
            'development' => true, 
            'bookmarks' => true, 
            'about' => true
        ),
        'Money' => array(
            'account.money' => true, 
            'account.store' => true, 
            'account.lottery' => true, 
            'users.top.money' => true
        ),
        'Account' => array('account' => true),
        'Mail' => array(
            'mail.inbox' => true, 
            'mail.sentbox' => true, 
            'mail.trash' => true, 
            'bulletins.post' => true, 
            'bulletins.my' => true, 
            'bulletins' => true
        )
    );

    foreach($headers as $header => &$pages) {
        echo '<li class="';
        if(isset($pages[$page_name])) echo 'content';
        echo '"><a href=""><em>', $header, '</em></a></li>';
    }            

}

?>

Я не фанат смешивания кода с выводом, но это подойдет, например.

PHP подсказка дня: не используйте конкатенацию строк, если вы просто выводите строку

Answer 4

Вместо того, чтобы использовать имена страниц в качестве ключей массива, вы можете просто получить массивы имен страниц, а затем сравнить, используя in_array ($page_name, $array), а не isset($array[$page_name]).

Это, к счастью, должно работать вместе с изменениями из @meager и позволит немного сжаться статическим битам кода в верхней части.

Answer 5

Объедините ваши массивы или поместите всю эту логику в другую хорошо названную функцию. Мой пример объединяет массивы.

// it's ugly, but at least the ugliness
// is confined to only _one_ array ;)
$map_pages_to_navitem = array(
    'home' => 'home',
    'users.online' => 'users',
    'users.online.male' => 'users',
    'users.online.female' => 'users',
    'users.online.friends' => 'users',
    'users.location' => 'users',
    'users.featured' => 'users',
    'users.new' => 'users',
    'users.browse' => 'users',
    'users.search' => 'users',
    'users.staff' => 'users',
    'forum' => 'forum',
    'widgets' => 'more',
    'news' => 'more',
    'promote' => 'more',
    'development' => 'more',
    'bookmarks' => 'more',
    'about' => 'more',
    'account.money' => 'money',
    'account.store' => 'money',
    'account.lottery' => 'money',
    'users.top.money' => 'money',
    'account' => 'account'),
    'mail.inbox' => 'mail', 
    'mail.sentbox' => 'mail', 
    'mail.trash' => 'mail', 
    'bulletins.post' => 'mail', 
    'bulletins.my' => 'mail', 
    'bulletins' => 'mail', 
);
$current = $map_pages_to_navitem[$page_name];

echo '<li class="'.($current=='home')?'current':''.'"><a href=""><em>Home</em></a></li>';
echo '<li class="'.($current=='users')?'current':''.'"><a href=""><em>Users</em></a></li>';
echo '<li class="'.($current=='forum')?'current':''.'"><a href=""><em>Forum</em></a></li>';
echo '<li class="'.($current=='more')?'current':''.'"><a href=""><em>More</em></a></li>';
echo '<li class="'.($current=='money')?'current':''.'"><a href=""><em>Money</em></a></li>';
echo '<li class="'.($current=='account')?'current':''.'"><a href=""><em>Account</em></a></li>';
echo '<li class="'.($current=='mail')?'current':''.'"><a href=""><em>Mail</em></a></li>';

Глядя на код, я также вижу конечный результат - присвоить элементу <li> значение атрибута класса. JavaScript сделает это лучше, чем PHP.

Таким образом, вы можете присвоить каждому <li> идентификатор и оставить присвоение атрибута класса JavaScript:

echo '<li id="home"><a href=""><em>Home</em></a></li>';
echo '<li id="users"><a href=""><em>Users</em></a></li>';
echo '<li id="forum"><a href=""><em>Forum</em></a></li>';
echo '<li id="more"><a href=""><em>More</em></a></li>';
echo '<li id="money"><a href=""><em>Money</em></a></li>';
echo '<li id="account"><a href=""><em>Account</em></a></li>';
echo '<li id="mail"><a href=""><em>Mail</em></a></li>';

echo '<script type="text/javascript">';
    echo 'document.getElementById("'.$current.'").className = "current";';
          // you'll want to sanitize $current to avoid parse errors in your JS
echo '</script>'

Answer 6

Если не изменить «вернуть htmlcode» из «печати htmlcode», я бы по крайней мере добавил для этого второй параметр.

Answer 7

Я не вижу веских причин для изменения исходного кода. Это очень легко читать и понимать, и достаточно легко изменить. Я также не думаю, что есть какая-либо причина использовать Javascript для установки индикатора вкладки. Используя PHP, вы обслуживаете людей, у которых отключен JavaScript, и мне нравится сохранять Javascript, когда это действительно необходимо.

Answer 8

    // set variables if there array value exist
if (isset($header_home[$page_name])){
    $current_home = 'current';
}else if (isset($header_users[$page_name])){
    $current_users = 'current';
}else if (isset($header_forum[$page_name])){
    $current_forum = 'current';
}else if (isset($header_more[$page_name])){
    $current_more = 'current';
}else if (isset($header_money[$page_name])){
    $current_money = 'current';
}else if (isset($header_account[$page_name])){
    $current_account = 'current';
}else if (isset($header_mail[$page_name])){
    $current_mail = 'current';
}

Вы можете использовать переменную-переменную (http://www.php.net/manual/en/language.variables.variable.php), foreach и break, чтобы уменьшить эту часть функции

Answer 9

Используйте switch вместо расширенного if/else для начинающих: -)