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!!!!!!

• Chris Gerken: +1 for admitting it's for a class
• SomeShinyObject: +1 for wanting to learn and not just get an answer.
• assylias: you could add a printQueriedItem method to your Catalog class, that would do the null check and print.
• Powerslave: @assylias I'd rather not do that. Catalog is a domain-level data repository and should not be mixed up with representation code.
• Eric: @user2355058 could you rollback your edit? We'd like to be able to save this question for posterity, and it's hard for the answer to be useful without the context of your question.

Answer 1

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);

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);

• CPerkins: Once you remove the class-level definition of queriedItem, yes - but just define it inside find.
• user2355058: This is my first time on Stackoverflow. That last commend was the result of a trivial naming error on my part xD.
• user2355058: Thank you so much CPerkins for helping me learn something that I could not learn by through reading my book or Google. You are a gentleman and a scholar.
• CPerkins: You are very welcome. Ahmed also gave you a good answer, incidentally. Good luck in your studies.

Answer 3

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.

• AndyPerfect: This is unnecessarily complex for someone who is just beginning with the language.
• user2355058: Yeah dude, this is clearly a java 101 question. Thanks for your long and detailed answer but it is counter productive due to my skill level and that of people who might find this through Google.
• Powerslave: @AndyPerfect Well, not really. That's why I suggested to "choose complexity at your own discretion". Its modularity is a key to the powers of Java. Also, the question originally read "how to do it right". This is how to do it right.
• Powerslave: @user2355058 If you simply want to return with the correct Item, instead of storing it in a public variable, let queriedItem be a local variable of the method find instead of a field and return it. Also note that you risk a NullPointerException in find as insert does not prohibit adding null values to the array and itemlist[posn].getKey() may resolve to null.getKey().