1

I have the following code in Product Implementation

public void updateProduct(Product product) throws ProductNotFoundException {

    Iterator<Product> it = allProducts.iterator();
    Product p = null;
    if (!allProducts.isEmpty()) {
        while (it.hasNext()) {
            p = it.next();
            if (p.getId() == product.getId()) {
                p.setPrice(product.getPrice());
                System.out.println("Successfully updated the product "
                        + product.getId());
            }
        }
    } else {
        System.out.println("No such product " + product.getId());
        throw new ProductNotFoundException(product.getName());
    }
}

// Main

ipod.setId(Integer.MAX_VALUE);

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

The Junit is throwing following exception

Exception in thread "main" productdb.util.AssertionFailedError: should've gotten ProductNotFoundException
    at productdb.util.Assert.fail(Assert.java:43)
    **at productdb.ProductDBClient.testProductServer(ProductDBClient.java:85)**
    at productdb.ProductDBClient.main(ProductDBClient.java:20)

Unfortunately, I am unable to use try and catch. Compiler is throwing error saying it cannot be caught.

Can you please provide me some pointers where I am doing incorrectly?

Thanks very much!!

4
  • Seems like you didn't put the code properly. Where's "allProducts" defined?
    – RaceBase
    May 31, 2013 at 5:33
  • 1
    I find this question somewhat unclear. You say that a compilation error occurs, yet you also include an exception thrown at runtime, indicating that the code must have compiled successfully (to be executed).
    – FThompson
    May 31, 2013 at 5:34
  • Sounds like you expect that the exception will be thrown when iPod product can't be found, but it's actually only thrown if allProducts is empty (which I'm guessing it's not).
    – jerry
    May 31, 2013 at 5:36
  • Reddy, allProducts is defined at the start, private static List<Product> allProducts = new ArrayList<Product>();
    – Newbie
    Jun 1, 2013 at 4:32

4 Answers 4

3

You forgot the case when for every available product p.getId() == product.getId() is false. Following the general code snippet logic you should also throw an exception if no id matches the request. Rewrite it like this:

    public void updateProduct(Product product) throws ProductNotFoundException {
    for (Product existing : allProducts) {
        if (existing.getId() == product.getId()) {
            existing.setPrice(product.getPrice());
            return;
        }
    }
    System.out.println("No such product " + product.getId());
    throw new ProductNotFoundException(product.getName());
}

You may also take a look at some functional-style library, like Guava, to ease operations on collections.

0
1

I didn't see any reason why it will throw "ProductNotFoundException"

if (!allProducts.isEmpty()) {
        //Code
} else {
        System.out.println("No such product " + product.getId());
        throw new ProductNotFoundException(product.getName());
}

Obviously if your allProducts is not empty, it will never throw the exception

Correct version of your code [Assuming if you are unable to update product, throw exception]

public void updateProduct(Product product) throws ProductNotFoundException {

    Iterator<Product> it = allProducts.iterator();
    Product p = null;
    if (!allProducts.isEmpty()) {
        while (it.hasNext()) {
            p = it.next();
            if (p.getId() == product.getId()) {
                p.setPrice(product.getPrice());
                System.out.println("Successfully updated the product "
                        + product.getId());
                return;
            }
        }
    } 
    throw new ProductNotFoundException(product.getName());
}
0
0

with the code you have, it is difficult to give a definitive answer. The problem is that the else path of the code isn't reached, you should use a debugger to find out why.

however certain tips to solve the problem:

1) change if (p.getId() == product.getId()) { to if (p.getId().equals(product.getId())) { I think your intention is to check for the equality of the id object rather than their reference.

2) step through the debugger to see if the else is reached, alternatively for starters to validate that try-catch is working just comment out the code in if which will give you the confidence that try-catch is working and your code stays in the if path and hence doesn't throw the exception

1
  • This is invaluable input. Thanks a lot
    – Newbie
    Jun 1, 2013 at 4:31
0

The reason is, you are never throwing the exception even if a product is not found among the existent. You only throw the exception when you don't have any product in. Try this code:

public void updateProduct(Product product) throws ProductNotFoundException {

    Iterator<Product> it = allProducts.iterator();
    Product p = null;
    if (!allProducts.isEmpty()) {
        while (it.hasNext()) {
            p = it.next();
            if (p.getId() == product.getId()) {
                p.setPrice(product.getPrice());
                System.out.println("Successfully updated the product "
                        + product.getId());
            }
        }
    }

    if (p == null ) {
        System.out.println("No such product " + product.getId());
        throw new ProductNotFoundException(product.getName());
    }
}
1
  • Thanks for your input. It was still throwing the exception.
    – Newbie
    Jun 1, 2013 at 4:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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