3
\$\begingroup\$

This class, UnorderedList, implements an Unordered list in Java. In this class, duplicates are allowed. remove(argument) will delete the first occurrence of the argument. I was wondering to add other functions named add(int index, int data) and set(int index, int data) or not. Since the class is unordered, I thought I would not need it. The size of the class is limited although that does not concern me (UnorderedLinkedList coming up next).

I tried to make sure all the functionality and behaviours are correct. If this class is lacking any method(s) and/or correct usage of exception and anything else please comment below. Any kind of suggestions and advice is appreciated.

//Node class

package lists.arrayLists.unorderedList;

public class Node
{
    private int data;

    public Node(int data)
    {
        this.data = data;
    }

    public int getData()
    {
        return data;
    }
}

//UnorderedList class

package lists.arrayLists.unorderedList;

import java.lang.IndexOutOfBoundsException;
import java.util.NoSuchElementException;

public class UnorderedList
{
    public static final int MAX_SIZE = 100;
    private int size;
    private Node[] nodeElements;
    private int currentIndex;                           // to navigate/iterate over list

    public UnorderedList()
    {
        nodeElements = new Node[MAX_SIZE];
    }

    public boolean isFull()
    {
        return (size() == MAX_SIZE);
    }

    public boolean isEmpty()
    {
        return (size() == 0);
    }

    public int size()
    {
        return size;
    }

    public boolean add(int data)
    {
        try
        {
            Node node = new Node(data);
            nodeElements[size++] = node;
            return true;
        }
        catch (IndexOutOfBoundsException ex)
        {
            System.out.println(ex.getMessage());
            throw new IndexOutOfBoundsException();
        }
    }

    public void remove(int data)
    {
        int index = locate(data);
        try
        {
            nodeElements[index] = nodeElements[size() - 1];
            size--;
        }
        catch (IndexOutOfBoundsException ex)
        {
            throw new NoSuchElementException();
        }
    }

    public boolean find(int data)
    {
        boolean found = false;
        try
        {
            if(locate(data) >= 0)
            {
                found = true;
            }
            return found;
        }
        catch (NoSuchElementException ex)
        {
            throw new NoSuchElementException();
        }
    }

    // locate function looks for an element in the array, if there is one then returns its index
    private int locate(int data)
    {
        int i = 0;
        int index = -1;
        boolean found = false;
        while (i < size() && !found)
        {
            if (data == nodeElements[i].getData())
            {
                found = true;
                index = i;
            }
            i++;
        }
        return index;
     }

    //Navigator methods

    public void reset()
    {
        currentIndex = 0;
    }

    public boolean hasNext()
    {
        return (currentIndex == size());
    }

    public void next()
    {
        if (!hasNext()) currentIndex++;
    }

    public int currentElement()
    {
        return nodeElements[currentIndex].getData();
    }
}
\$\endgroup\$
2
\$\begingroup\$
  • locate never throws, so what find expects to catch? Consider

    public boolean find(int data) {
        return locate(data) >= 0;
    }
    
  • locate is overcomplicated. Consider

    private int locate(data) {
        for (int index = 0; index < size(); index++) {
            if (data == nodeElements[i].getData()) {
                return index;
            }
        }
        return -1;
    
  • reset, hasNext, and next do not belong to the list class; they are the methods of a list iterator. For starters, notice that your implementation only allows one list traversal at a time.

    That said, the semantics of hasNext() is wrong. The condition currentIndex == size() means that there is no next. A client of your class would be badly confused.

    Along the same line, next() should not silently ignore the end-of-list condition.

\$\endgroup\$
  • \$\begingroup\$ how do I separate Iterator class from this class? Any demonstration or link to this solution would be very helpful. @vnp \$\endgroup\$ – Hamidur Rahman Jun 8 '18 at 19:06
  • \$\begingroup\$ @HamidurRahman I don't know Java conventions, but normally you'd define an iterator class holding currentIndex, with next and hasNext methods, and pass your list to its constructor, \$\endgroup\$ – vnp Jun 8 '18 at 19:09
  • \$\begingroup\$ Thanks. I will look for it and find the solution and then post it in this comment section. Is that okay? @vnp \$\endgroup\$ – Hamidur Rahman Jun 8 '18 at 19:12
  • \$\begingroup\$ @HamidurRahman Better post a follow-up question. \$\endgroup\$ – vnp Jun 8 '18 at 19:13
2
\$\begingroup\$

I would recommend using Log4j instead of System.out.println() with some meaningful message to the user. Also, you can implement locate() method as mentioned below to avoid initializing the flag

private int locate(int data)
{
    int i = 0;
    int index = -1;
    while (i < size())
    {
        if (data == nodeElements[i].getData())
        {
           index = i;
           break;
        }
        i++;
    }
    return index;
 }

Change: Use break; to come out of the loop rather than using flag to check it.

\$\endgroup\$
  • \$\begingroup\$ Please summarize the changes you made. Code only answers are considered off-topic. \$\endgroup\$ – t3chb0t Jun 8 '18 at 6:50
  • 1
    \$\begingroup\$ @t3chb0t, done:). \$\endgroup\$ – VijayD Jun 8 '18 at 7:27

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.