HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Directory that lists employee information - follow-up

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
directoryemployeethatfollowlistsinformation

Problem

Yesterday I posted a question involving multiple nested queries. The queries pulled information from the database and created a directory listing of all employees. There are two many-to-many relationships involving 5 tables. For each employee they can have multiple job titles and multiple departments.

$stmt = $mysqli->prepare("SELECT employeeId, firstName, middleName, lastName, suffix, profilePhoto, GROUP_CONCAT(DISTINCT jobName ORDER BY jobTitleId), GROUP_CONCAT(DISTINCT departmentName ORDER BY departmentId), GROUP_CONCAT(DISTINCT departmentURL ORDER BY departmentId) FROM employee 
INNER JOIN employee_has_jobTitle ON employeeId = employee_has_jobTitle.employee_employeeId 
INNER JOIN jobTitle ON employee_has_jobTitle.jobTitle_jobTitleId = jobTitleId 
INNER JOIN employee_has_department ON employee.employeeId = employee_has_department.employee_employeeId
INNER JOIN department ON employee_has_department.department_departmentId = departmentId
GROUP BY employeeId ORDER By lastName, firstName");

$stmt->execute();
$stmt->store_result();
$stmt->bind_result($employeeId, $firstName, $middleName, $lastName, $suffix, $profilePhoto, $jobName, $departmentName, $departmentURL);

//set up the list
echo "";

while($stmt->fetch()){

    echo "";

    $deptName = explode(",", $departmentName);
    $deptURL = explode(",", $departmentURL);
    $jobTitle = explode(",", $jobName);
    echo "$firstName $lastName";

    $counter = 0;
    foreach($deptName as $name) {
        //echo a preceding comma if not the first department
        if($counter > 0) echo ", ";

        echo "$name";
        $counter++;
    }

    echo "";

    foreach($jobTitle as $job) {
        echo "$job";
    }

    echo "";
}

//close the list
echo "";


If you check my other question, you can see that this is an improvement. However, I don't think the query and code are as elegant as they should be. Is there an obvious way to simplify the code and enhance performance that I'm not seeing?

Screenshot of

Solution

in your while statement you are listing all sorts of information inside of one list item tag, I am not so sure this is intentional, it looks like it is going to be really messy. I think you should look into creating sub-lists there especially for the multiple Department names and Job Names. you want something like an XML Date File here, maybe I am overstepping my reviewer line and telling you to change the structure, but this is going to look really messy on a website I think.

something like this would be more convenient.


    
       
       $firstName $lastName 
       
           Department1
           Department1
       
       
           Job1
           Job2
       
    


I don't know exactly what you want your finished product to look like, but this structure is far more solid than what your PHP is going to output, you should look into making it output something like this, because this is going a lot easier to work with for Styling and manipulating with PHP.

all of your image tags (`) need to be closed(), so does your break tags (
,
`)

From what you are saying it sounds like you want something more like this


    
        
            
        
        
            $firstName $lastName 
        
        Department1, Department1
        Job1, Job2
    


I removed the Div tag and some of the other stuff just so we could visualize this better. this is a little more organized than what you are going to get with your current code. it will still be easier to deal with.

As Far as whether or not to do another list for the Department Names and Job Titles, I would go with yes, even if they only have a single item, it will be easier to navigate the ones that do have them, and easier to manipulate the data, it is more maintainable I think.

I mentioned still using nested lists for the Department Names and Job Names, you should still have a title for the list item that is holding them, so you know what they are, but I will leave that up to you


    
        
            
        
        
            $firstName $lastName 
        
        
            
                Department1
                Department1
            
        
        
                        
                Job1
                Job2
            
        
    


so here is what that would look like with out the titles for the sub lists

Code Snippets

<li>
    <div class='clearfix'>
       <img src='/profileImages/$profilePhoto' alt='$firstName $lastName' class='directoryPhoto' />
       <strong>$firstName $lastName</strong> <!-- should probably be a span with styling hooks -->
       <ul>
           <li>Department1</li>
           <li>Department1</li>
       </ul>
       <ul>
           <li>Job1</li>
           <li>Job2</li>
       </ul>
    </div>
</li>
<li>
    <ul>
        <li>
            <img src='/profileImages/$profilePhoto' alt='$firstName $lastName' class='directoryPhoto' />
        </li>
        <li>
            <strong>$firstName $lastName</strong> <!-- should probably be a span with styling hooks -->
        </li>
        <li>Department1, Department1</li>
        <li>Job1, Job2</li>
    </ul>
</li>
<li>
    <ul>
        <li>
            <img src='/profileImages/$profilePhoto' alt='$firstName $lastName' class='directoryPhoto' />
        </li>
        <li>
            <strong>$firstName $lastName</strong> <!-- should probably be a span with styling hooks -->
        </li>
        <li>
            <ul>
                <li>Department1</li>
                <li>Department1</li>
            </ul>
        </li>
        <li>
            <ul>            
                <li>Job1</li>
                <li>Job2</li>
            </ul>
        </li>
    </ul>
</li>

Context

StackExchange Code Review Q#47358, answer score: 4

Revisions (0)

No revisions yet.