patternphpMinor
Directory that lists employee information - follow-up
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.
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
$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.
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 (`
`)
From what you are saying it sounds like you want something more like this
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
so here is what that would look like with out the titles for the sub lists
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.