Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

The following example shows how to implement a queue in an employee structure. I am looking for a review of the implementation:

import java.util.LinkedList;

class GenQueue<E> {
   private LinkedList<E> list = new LinkedList<E>();
   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<? extends E> q) {
      while (q.hasItems())
         list.addLast(q.dequeue());
   }
}

Here's a test class:

public class GenQueueTest {
   public static void main(String[] args) {
      GenQueue<Employee> empList;
      empList = new GenQueue<Employee>();
      GenQueue<HourlyEmployee> hList;
      hList = new GenQueue<HourlyEmployee>();
      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 
share|improve this question

1 Answer 1

up vote 12 down vote accepted

!The Queue

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<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());
   }
}

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.

share|improve this answer
    
Thanks for your comments and explaining/showing an example of what I can do to make it better and more clear as a queue. –  splashinn Jun 12 at 19:33
2  
Extending LinkedList makes your GenQueue more vurnerable, as methods from List can be called. Better to use encapsulation if you want to ensure proper behaviour. –  RobAu Jun 13 at 7:44

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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