Advice on Java program

Question

My java project required that I create an array of objects(items), populate the array of items, and then create a main method that asks a user to enter the item code which spits back the corresponding item.

It took me a while to figure out, but I ended up "cheating" by using a public variable to avoid passing/referencing the object between classes.

Please help me properly pass the object back.

This is the class with most of my methods including insert and the find method.

public class Catalog {
    private Item[] itemlist;
    private int size;
    private int nextInsert;
    public Item queriedItem;

    public Catalog (int max) {

        itemlist = new Item[max];
        size = 0;
    }
    public void insert (Item item) {
        itemlist[nextInsert] = item;
        ++nextInsert;
        ++size;
    }
    public Item find (int key) {
        queriedItem = null;

        for (int posn = 0; posn < size; ++posn) {
            if (itemlist[posn].getKey() == key) queriedItem = itemlist[posn];
        }{
            return queriedItem;
        }
    }
}

This is my main class:

import java.util.*;

public class Program {
    public static void main (String[] args) {

        Scanner kbd = new Scanner (System.in);
        Catalog store;
        int key = 1;

        store = new Catalog (8);
        store.insert(new Item(10, "food", 2.00));
        store.insert(new Item(20, "drink", 1.00));



        while (key != 0) {

            System.out.printf("Item number  (0 to quit) ?%n");
            key = kbd.nextInt();
            if (key == 0) {
                System.out.printf("Exiting program now!");
                System.exit(0);
            }

            store.find(key);

            if (store.queriedItem != null) {
                store.queriedItem.print();
            }
            else System.out.printf("No Item found for %d%n", key);

        }
    }
}

Thanks I appreciate the help!!!!!!

Answer 1

Well, here are some suggesetions (choose complexity at your own discretion, but all of them is highly recommended):

• Research Properties, for example here. Or XML. You could populate the array with values from a configuration file for greater flexibility.
• Use constanst for literals in your code (where they are necessary).
• Create an ApplicationFactory the initializes the whole application for you. Things like this need to be separated from your domain logic.
• Create a UserInputProvider interface so you can easily change the way the input of the user is read without affecting anything else. Implement it with a ConsoleInputProvider class for example.
• In general, try using interfaces for everything that's not a pure domain object (here, the only one you have is probably Item).
• Try to keep your methods as short as possible. Instead of doing many things in a method, have it invoke other methods (grouping related logic) named appropriately to tell what it is doing.
• If you're not allowed to cheat and use List or a Map, devise your own implementation of one, separating data structure and handling from the logic represented by Catalog (i.e. Catalog in turn will delegate to, for example, Map.get or equivalent method of your data structure implementation)
• Your main should basically just have ApplicationFactory (or an IoC framework) to build and initialize your application, invoke the UserInputProvider (it should not know the exact implementation it is using) to get user input, validate and convert the data as required, invoke Catalog to find the appropriate Item and then (similarly to the input interface) send the result (the exact data it got, not some string or alike) to some implementation of a SearchResultView interface that decides how to display this result (in this case it will be a console-based implementation, that prints a string representing the Item it got).

Generally, the higher the level of decoupling you can achieve, the better your program will be.

The Single Responsibility Principle states: " every class should have a single responsibility, and that responsibility should be entirely encapsulated by the class". This is also true for methods: they should have one and only one well defined task without any side effects.

Answer 2

Remove your use of queriedItem entirely and just return the item from find: Replace

        store.find(key);

    if (store.queriedItem != null){store.queriedItem.print();}else System.out.printf("No Item found for %d%n", key);

With

Item foundItem = store.find(key);
if (foundItem != null) {
   foundItem.print();
} else System.out.printf("No Item found for %d%n", key);

Answer 3

store.find(key); returns an Item you should use it and delete the public field from Catalog

public Item find (int key) {
   Item queriedItem = null;
   //....
}

---

Item searched = store.find(key);

if (searched != null)
   searched.print();
else 
   System.out.printf("No Item    found for %d%n", key);