views:

280

answers:

4

I am trying to implement an OutOfStockException for when the user attempts to buy more items than there are available. I'm not sure if my implementation is correct. Does this look OK to you?

public class OutOfStockException extends Exception {


    public OutOfStockException(){
        super();
    }

    public OutOfStockException(String s){
        super(s);
    }
}

This is the class where I need to test it:

import javax.swing.JOptionPane;

public class SwimItems {
    static final int MAX = 100;


    public static void main (String [] args)
    {
        Item [] items = new Item[MAX];
        int numItems;

        numItems = fillFreebies(items);
        numItems += fillTaxable(items,numItems);
        numItems += fillNonTaxable(items,numItems);

        sellStuff(items, numItems);
    }

    private static int num(String which) {
        int n = 0;
        do {
                       try{
            n=Integer.parseInt(JOptionPane.showInputDialog("Enter number of "+which+" items to add to stock:"));
                       }
                       catch(NumberFormatException nfe){
                           System.out.println("Number Format Exception in num method");
                       }
        } while (n < 1 || n > MAX/3);
        return n;
    }

    private static int fillFreebies(Item [] list)
    {
        int n = num("FREEBIES");
        for (int i = 0; i < n; i++)
                    try{
            list [i] = new Item(JOptionPane.showInputDialog("What freebie item will you give away?"),
            Integer.parseInt(JOptionPane.showInputDialog("How many do you have?")));
                    }
                    catch(NumberFormatException nfe){
                        System.out.println("Number Format Exception in fillFreebies method");
                    }
                    catch(ArrayIndexOutOfBoundsException e){
                        System.out.println("Array Index Out Of Bounds Exception in fillFreebies method");
                    }

        return n;
    }

    private static int fillTaxable(Item [] list, int number)
    {
        int n = num("Taxable Items");
        for (int i = number ; i < n + number; i++)
                    try{
            list [i] = new TaxableItem(JOptionPane.showInputDialog("What taxable item will you sell?"),
                Double.parseDouble(JOptionPane.showInputDialog("How much will you charge (not including tax) for each?")),
                    Integer.parseInt(JOptionPane.showInputDialog("How many do you have?")));
                    }
                    catch(NumberFormatException nfe){
                        System.out.println("Number Format Exception in fillTaxable method");
                    }
                    catch(ArrayIndexOutOfBoundsException e){
                        System.out.println("Array Index Out Of Bounds Exception in fillTaxable method");
                    }

        return n;
    }


    private static int fillNonTaxable(Item [] list, int number)
    {
        int n = num("Non-Taxable Items");
        for (int i = number ; i < n + number; i++)
                    try{
            list [i] = new SaleItem(JOptionPane.showInputDialog("What non-taxable item will you sell?"),
                    Double.parseDouble(JOptionPane.showInputDialog("How much will you charge for each?")),
                    Integer.parseInt(JOptionPane.showInputDialog("How many do you have?")));
                    }
                    catch(NumberFormatException nfe){
                        System.out.println("Number Format Exception in fillNonTaxable method");
                    }
                    catch(ArrayIndexOutOfBoundsException e){
                        System.out.println("Array Index Out Of Bounds Exception in fillNonTaxable method");
                    }

        return n;
    }


    private static String listEm(Item [] all, int n, boolean numInc)
    {
        String list = "Items:  ";
        for (int i = 0; i < n; i++)
        {
                    try{
            list += "\n"+ (i+1)+".  "+all[i].toString() ;
            if (all[i] instanceof SaleItem) list += " (taxable) ";
            if (numInc) list += " (Number in Stock: "+all[i].getNum()+")";
                    }
                    catch(ArrayIndexOutOfBoundsException e){
                        System.out.println("Array Index Out Of Bounds Exception in listEm method");
                    }
                    catch(NullPointerException npe){
                        System.out.println("Null Pointer Exception in listEm method");
                    }

        }
        return list;
    }



    private static void sellStuff (Item [] list, int n) {

        int choice;
        do {
                    try{
            choice = Integer.parseInt(JOptionPane.showInputDialog("Enter item of choice:  "+listEm(list, n, false)));
                    }
                    catch(NumberFormatException nfe){
                        System.out.println("Number Format Exception in sellStuff method");
                    }

        }while (JOptionPane.showConfirmDialog(null, "Another customer?")==JOptionPane.YES_OPTION);

        JOptionPane.showMessageDialog(null, "Remaining "+listEm(list, n, true));

        }

}
+3  A: 

The implementation looks fine; you don't have to do much in an exception class. You might consider adding members for what the thing was that was out of stock, how many were requested, and how many there were in stock when the request was made so that code catching the exception has access to that information. So for instance, here I've a stock item code:

public class OutOfStockException extends Exception {

    private int stockCode;

    public OutOfStockException(int stockCode){
        super();
        this.stockCode = stockCode;
    }

    public OutOfStockException(String s){
        super(s);
        this.stockCode = stockCode;
    }

    public int getStockCode() {
        return this.stockCode;
    }
}

You could then create one like this:

throw new OutOfStockException(StockCodes.WIDGET, "Out of widgets");

But that's up to you, and at that point it's just class design like anything else.

Many times, with these sorts of things, I only include constructors with the individual parts, and then have the class itself generate the message for the underlying Exception getMessage message. So:

public class OutOfStockException extends Exception {

    private int stockCode;

    public OutOfStockException(int stockCode){
        super("Out of " + StockCodes.getDescription(stockCode));
        this.stockCode = stockCode;
    }

    public int getStockCode() {
        return this.stockCode;
    }
}

// Throwing one:
throw new OutOfStockException(StockCodes.WIDGETS);

But again it's just class design at that point.

All of that aside, and this is slightly off-topic, but being out of stock on an item seems to me to be a normal situation, not an exceptional one; are you sure an exception is really the right way to model it?

T.J. Crowder
A: 

Yes, your exception is correctly implemented.

However, I'd suggest including more information in it:

  • the product in question
  • the attempted number of items to order
  • the actual number of remaining items

for example, if the product "Bike" has 2 remaining items, but one tries to order three, the exception would be constructed and thrown like

throw new OutOfStockException(product, requestedQuantity, actualQuantity)
Bozho
A: 

Your exception looks ok.

I'm not sure about your SaleItem and TaxableItem classes though.

if (all[i] instanceof SaleItem) list += " (taxable) ";

is a code smell - having to check the instance of the class before doing something (and I'm not sure the names makes sense given the above). Why not override the appropriate methods on the class to do the above automatically for the appropriate subclass.

Brian Agnew
A: 

I disagree with the use of Exception here. You should only use one for exceptional conditions as the nomenclature suggests and not for control flow as this will make your code far more pleasant to read. Also exceptions do not get optimized byt the JVM and as such execute a lot slower.

Yiannis