patternjavaModerate
Queued Employees
Viewed 0 times
employeesqueuedstackoverflow
Problem
The following example shows how to implement a queue in an employee structure. I am looking for a review of the implementation:
Here's a test class:
And the
The output is:
import java.util.LinkedList;
class GenQueue {
private LinkedList list = new LinkedList();
public void enqueue(E item) {
list.addLast(item);
}
public E dequeue() {
return list.poll();
}
public boolean hasItems() {
return !list.isEmpty();
}
public int size() {
return list.size();
}
public void addItems(GenQueue q) {
while (q.hasItems())
list.addLast(q.dequeue());
}
}Here's a test class:
public class GenQueueTest {
public static void main(String[] args) {
GenQueue empList;
empList = new GenQueue();
GenQueue hList;
hList = new GenQueue();
hList.enqueue(new HourlyEmployee("T", "D"));
hList.enqueue(new HourlyEmployee("G", "B"));
hList.enqueue(new HourlyEmployee("F", "S"));
empList.addItems(hList);
System.out.println("The employees' names are:");
while (empList.hasItems()) {
Employee emp = empList.dequeue();
System.out.println(emp.firstName + " "
+ emp.lastName);
}
}
}And the
Employee class:class Employee {
public String lastName;
public String firstName;
public Employee() {
}
public Employee(String last, String first) {
this.lastName = last;
this.firstName = first;
}
public String toString() {
return firstName + " " + lastName;
}
}
class HourlyEmployee extends Employee {
public double hourlyRate;
public HourlyEmployee(String last, String first) {
super(last, first);
}
}The output is:
The employees' name are:
T D
G B
F S
Solution
!The Queue
About the not-the-queue things:
-
your Employee and HourlyEmployee classes have public fields (
-
Your code is generally well formatted. It's easy to read.
-
The variable names are appropriate.
Basically, this is decent code. It looks like the code does the job too.... now, about the job....
The Queue
What you have done is 'wrap' a LinkedList, and called it a queue. You have not actually implemented a queue. There is nothing wrong with doing this, but it does not add much value either. The confusion continues with your naming here.... For example, why is it called
Your generics are good on the
Unfortunately, it is 'bad practice' to modify the input value
Out of interest, it would be interesting to present your code as an extension to a LinkedList, instead of a wrapper. Consider this:
Now your GenQueue is a Queue (java.util.Queue) and it is also a List, and it supports all the features that are already embedded in those implementations.
About the not-the-queue things:
-
your Employee and HourlyEmployee classes have public fields (
firstname, etc.) You should instead have 'getters' for them (like getFirstName()). I would recommend making those values 'final' as well, but I don't think this is central to your question about queues.-
Your code is generally well formatted. It's easy to read.
-
The variable names are appropriate.
Basically, this is decent code. It looks like the code does the job too.... now, about the job....
The Queue
What you have done is 'wrap' a LinkedList, and called it a queue. You have not actually implemented a queue. There is nothing wrong with doing this, but it does not add much value either. The confusion continues with your naming here.... For example, why is it called
empList when it's a GenQueue?Your generics are good on the
addItems() though.Unfortunately, it is 'bad practice' to modify the input value
q in a method called addItems()... i.e. it should be called transferItems or something. I would expect a method like addItems to leave the items in the source, and copy them to the destination.Out of interest, it would be interesting to present your code as an extension to a LinkedList, instead of a wrapper. Consider this:
class GenQueue extends LinkedList {
public void enqueue(E item) {
addLast(item);
}
public E dequeue() {
return poll();
}
public boolean hasItems() {
return !isEmpty();
}
public void addItems(GenQueue q) {
while (q.hasItems())
addLast(q.dequeue());
}
}Now your GenQueue is a Queue (java.util.Queue) and it is also a List, and it supports all the features that are already embedded in those implementations.
Code Snippets
class GenQueue<E> extends LinkedList<E> {
public void enqueue(E item) {
addLast(item);
}
public E dequeue() {
return poll();
}
public boolean hasItems() {
return !isEmpty();
}
public void addItems(GenQueue<? extends E> q) {
while (q.hasItems())
addLast(q.dequeue());
}
}Context
StackExchange Code Review Q#54095, answer score: 11
Revisions (0)
No revisions yet.