5
\$\begingroup\$

I have a multidimensional array with associative values... I want to add some information to multiple arrays at the same time. Doing $array1=$array2=array(... seems to work but I'm worried that it's too easy. Will any array get extra data or have data overwritten by doing this? What kind of unexpected behavior might I expect from this kind of code? Al should have 3 entries in it, while Bob and Chris will each have two... right? It seems to work, but I'm worried I'm overlooking something.

<?    $content['Al'][]= array(
       "link"=>"/admin.php",
       "text"=>"Admin Login",
       "icon"=>"wp.png");
    $content['Al'][]=$content['Bob'][]=$content['Chris'][] = array(
       "link"=>"/Wordpress/wp-login.php",
       "text"=>"Login to Wordpress",
       "icon"=>"wp.png");
    $content['Al'][]=$content['Bob'][]=$content['Chris'][] = array(
       "link"=>"/Gallery/Login.php",
       "text"=>"Login to Gallery",
       "icon"=>"picutres.jpg");



function userbox($heading,$contents){
    if($contents=="") return;
    echo "<div class=\"refbox\">

            <h2>".$heading."</h2>
            <p>";

    foreach ($contents as $key){
        if($key['link']!="")echo "<a href=\"".$key['link']."\">";
        if($key['icon']!="")echo"<img src=\"/images/icons/".$key['icon']."\" alt=\"".$key['text']."\" class=\"icons\">";
        echo $key['text'];
        if($key['link']!="")echo "</a>";
        if($key['updated']!="")echo " <span style=\"float:right;\">(".$key['updated'][0].")</span>";
        echo "<br>";    
        }       
    echo "</p>
        </div>";    
}


?>

Then in the content of the page I have

<?userbox($user,$content[$user]);?>

This should show a 'refbox' div with the users links in it within the page.

\$\endgroup\$
  • \$\begingroup\$ It now makes much more sense, and appears to now be review-able. Watch the hypothetical words though, people see those and move on (Say I have...), instead focus on the actual concern you have. For example: "This code works, but it seems like a hack", "I'm worried my solution will be difficult to maintain / read", "It does what I expect, but seems to perform slowly, how could it be optimized", etc... Also, thanks for caring and taking the time to fix up the question. \$\endgroup\$ – xDaevax Nov 18 '14 at 15:20
  • \$\begingroup\$ Thanks for the suggestions! I've cleaned it up a bit further... Afraid I'm a bit used to Stackoverflow so posted the bare minimum. \$\endgroup\$ – aslum Nov 18 '14 at 15:26
1
\$\begingroup\$

It works and I can't see any side effects, but it is unusual to see code like this, and I am not sure of the point of creating all 3 user arrays at once.

Do you intend to display all 3 user menu options at the same time, or can only 1 user be logged in at once?

This is my preference for readability, also if you create a menu array and need items in a different order for different users, it would be difficult to achieve using the current code.

<?php

function build_menu($user)
{
    // ### clearly show what each test is and what menu options are being set
    $menu = array();

    if ($user == 'Al') {
        $menu[] = array(
            "link" => "/admin.php",
            "text" => "Admin Login",
            "icon" => "wp.png");
    }

    if (in_array($user, array('Bob', 'Chris'))) {
        // menu options for multiple users can use in_array test
    }

    // default menu options for all users
    $menu[] = array(
        "link" => "/Wordpress/wp-login.php",
        "text" => "Login to Wordpress",
        "icon" => "wp.png");

    $menu[] = array(
        "link" => "/Gallery/Login.php",
        "text" => "Login to Gallery",
        "icon" => "picutres.jpg");

    return $menu;
}


function userbox($heading, $contents)
{
    // ### contents is supposed to be an array, why test it for being a blank string?
    // ### use {} to show scope
    if ($contents == "") {
        return;
    }

    // ### escape heading, what if it has html chars
    echo "<div class=\"refbox\">

            <h2>" . htmlspecialchars($heading). "</h2>
            <p>";

    /// ### think about your variable naming, $contents is actually menu items
    /// ### and $key is ambiguous, key of what?
    /// ### what if you used foreach ($menu_items as $menu_item) {

    foreach ($contents as $key) {

        // ### use {} around if statements, it is easier to see the scope of what is effected
        // ### use not empty instead of != "", that way if an array key doesn't exist like the 'updated' key below, your program won't generate a warning
        // ### use single quotes if you don't want to escape every double quote

        // if($key['link']!="")echo "<a href=\"".$key['link']."\">";
        if (!empty($key['link'])) {
            echo '<a href="' . $key['link'] . '">';
        }

        // ### i have given an example above, I will leave these ones for you to change
        if($key['icon']!="")echo"<img src=\"/images/icons/".$key['icon']."\" alt=\"".$key['text']."\" class=\"icons\">";

        // ### escape html incase there are special characters
        // echo $key['text'];
        echo htmlspecialchars($key['text']);

        if($key['link']!="")echo "</a>";
        if($key['updated']!="")echo " <span style=\"float:right;\">(".$key['updated'][0].")</span>";
        echo "<br>";
    }
    echo "</p>
        </div>";
}

$user = 'Al';
$menu = build_menu($user);
userbox($user, $menu);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.