CircAnalyzer
CircAnalyzer

Reputation: 704

JAVA update ArrayList object method not updating

I have an interface test class that implements another super-class. In the test class I have a method that is supposed to update an object from an array list; first it's supposed to check if the objects is in the list, if it is, it will delete and add a new object (replace). If it cant find the object it will throw an exception message. Here is the code I have implemented:

public class ProductDBImpl implements ProductDB {

// field declarations
ArrayList<Product> products = new ArrayList<Product>();
@Override
public void updateProduct(Product product) throws ProductNotFoundException 
{
    // TODO Auto-generated method stub
    Iterator<Product> pritr = products.iterator();
    while(pritr.hasNext())
    {
        Product pr = pritr.next();

        if (!pr.getId().equals(product.getId()))
        {

            throw new ProductNotFoundException("Product does no exist");

        }
        pritr.remove();

    }
    products.add(product);

}

First off I dont know if this is the correct way to do it. And, when I test it with my test client script, I get an error that reads:

Exception in thread "main" productdb.util.AssertionFailedError: should've gotten ProductNotFoundException

The code for the test client is as follows:

    ipod.setId(Integer.MAX_VALUE);
    try {
        productDB.updateProduct(ipod);
        Assert.fail("should've gotten ProductNotFoundException");
    } catch (ProductNotFoundException pnfe) {
        // expecting this
    }

Please help me identify my error. Thanks.

LATEST EDIT I updated my code based on the feedback I was getting RE the first item throwing the error:

public void updateProduct(Product product) throws ProductNotFoundException 
    {
        // TODO Auto-generated method stub
        Iterator<Product> pritr = products.iterator();
        while(pritr.hasNext())
        {
            Product pr = pritr.next();
            System.out.println(pr.getId());
            System.out.println(product.getId());
            if (pr.getId().equals(product.getId()))
            {

                pritr.remove();

            }
            else
            {
                throw new ProductNotFoundException("Product Not Found");
            }

        }
        products.add(product);

    }

ANOTHER UPDATE

public void updateProduct(Product product) throws ProductNotFoundException 
{
    // TODO Auto-generated method stub
    Iterator<Product> pritr = products.iterator();
    boolean match = true;
    while(pritr.hasNext())
    {
        Product pr = pritr.next();
        if (pr.getId().equals(product.getId()))
        {

            pritr.remove();

        }
        else
        {
            match = false;
        }

    }
    if (match == false)
    {
        new ProductNotFoundException("Product not found");
    }
    else
    {
        products.add(product);
    }


}

Upvotes: 2

Views: 2510

Answers (2)

Hovercraft Full Of Eels
Hovercraft Full Of Eels

Reputation: 285403

Again, walk through your logic. You have this:

public void updateProduct(Product product) throws ProductNotFoundException 
{
    // TODO Auto-generated method stub
    Iterator<Product> pritr = products.iterator();
    boolean match = true;
    while(pritr.hasNext())
    {
        Product pr = pritr.next();
        if (pr.getId().equals(product.getId()))
        {

            pritr.remove();

        }
        else
        {
            match = false;
        }

    }
    if (match == false)
    {
        new ProductNotFoundException("Product not found");
    }
    else
    {
        products.add(product);
    }


}

which translates to:

start method
    set match to true
    while loop
        if product found remove original product
        else match is set to false  // this will always happen one or more times!
    end while loop

    // match is almost guaranteed to be false!
    check match. if false, throw exception
    else if true, add new product
end method

Now assume that the data is:

No match  
No match  
No match  
match  
No match  
No match

And see what happens. With your logic the boolean will end up false when it shouldn't be.

Compare this with data that should throw the exception:

No match  
No match  
No match  
No match  
No match  
No match

And again see what happens.

What you want:

start method
    set match to false
    while loop
        if product found 
             remove original product
             replace with new product
             set match to true
        // no else block needed
    end while loop

    check match. if false, throw exception
    else if true, add new product
end method

or

start method
    // no need for match variable
    while loop
        if product found 
             remove original product
             replace with new product
             set match to true
             return from method // *****
    end while loop

    // if we reach this line, a match was never found
    throw exception

end method

Upvotes: 2

Niklas P
Niklas P

Reputation: 3507

The Assert.fail is executed when no Exception is thrown and this is the case if:

  • either the first (this seems to be an unwanted error!) element of the list (products) has an equal id
  • or the list (products) is empty

What's the content of the list?

Upvotes: 2

Related Questions