views:

2136

answers:

8

I'm working on a java program, and I have several vectors defined and filled (from a file) inside a method. I need to return the contents of all the vectors from the method. I have heard you can put them all in one object to return them. Is that possible, and if so, how? If not, do you have any possible solutions for me? Thanks in advance for your help!

Here is a code snippet:

Object getInventory()
{       
        Vector<String> itemID=new Vector<String>();
        Vector<String> itemName=new Vector<String>();
        Vector<Integer> pOrdered=new Vector<Integer>();
        Vector<Integer> pInStore=new Vector<Integer>();
        Vector<Integer> pSold=new Vector<Integer>();
        Vector<Double> manufPrice=new Vector<Double>();
        Vector<Double> sellingPrice=new Vector<Double>();  
        Object inventoryItem=new Object(); //object to store vectors in

    try
    {
        Scanner infile= new Scanner(new FileReader("Ch10Ex16Data.txt"));

        int i=0;

        while (infile.hasNext())
        {                
            itemID.addElement(infile.next());                
            itemName.addElement(infile.next()+infile.nextLine());
            pOrdered.addElement(infile.nextInt());
            pInStore.addElement(pOrdered.elementAt(i));
            pSold.addElement(0);
            manufPrice.addElement(infile.nextDouble());
            sellingPrice.addElement(infile.nextDouble());
            i++;

        }
        infile.close();

        System.out.println(itemID);
        System.out.println(itemName);
        System.out.println(pOrdered);
        System.out.println(pInStore);  
        System.out.println(pSold);
        System.out.println(manufPrice);
        System.out.println(sellingPrice);

    }
    catch (Exception f)
    {
       System.out.print(f);
    }

     return inventoryItem;
}
+3  A: 

First of all, use ArrayList instead of Vector. Then use a Map as your return object, with each value of the entry is one of your Lists.

Second of all, a much better approach is to create an object that actually holds each of your fields and return a java.util.List of these objects.

public class Item
{
    String id;
    String name
    Integer pOrdered;        
    Integer inStore;
           :
           :
Robin
Yeah, about the ArrayList, is that a "relatively new" addition to Java? Because my (not good) book does not talk about them... Hmmm... I definitely need a new Java book...Thanks for your help!
Danielle
It was added in J2SE 1.5, so... 2004? Something like that.
Adam Jaskiewicz
ArrayList was added in 1.2, December of 1998 (wow! that long?!). It should be covered under a section called "Java Collections".
James Schek
D'oh, got it mixed up with all the enhancements to the collections stuff in 1.5.How old is your Java book? Can you please smack your teacher with it for using a 10+ year old book to teach a Java course?
Adam Jaskiewicz
hey, some of us are stuck writing code that works in Java 1.1 (bloody MS JVM...). We don't get any of the new toys...
Herms
Sorry, but all I can say to that is... eeeeeeeeeeeeeeeeeww.
Adam Jaskiewicz
+8  A: 

Personnally, I'd scrap that approach completely. It seems like you need a Product class:

public class Product {

    private String itemName;
    private int itemID;
    // etc etc

    public Product(String itemName, int itemID) {
       this.itemName = itemName;
       this.itemID = itemID;
       // etc etc
     }

    public String getItemName() {
       return itemName;
    }

     public int getItemID() {
      return itemID;
    } 

    // etc etc
}

Then something like this :

public class Invertory {

 private List<Product> products = new ArrayList<Product>
 // etc etc

public Inventory(String fileName) throws IOException {
      // Load file,
       // Read each product, 
       products.add(new Product(...product arguments); //add to array
  }

  public Product[] getProducts() {
      return products.toArray(new Product[]{});
  }

}

Richie_W
Thank you for the detailed description, I will probably refer to it to get this done, thanks! Like I said, still new to Java, so getting some of it confused...
Danielle
+1  A: 

You really should reconsider your design here. You have multiple vectors, each with properties of the same type of thing — an item in your inventory. You should probably turn this into a single class, perhaps InventoryItem, with members for the name, price, etc. Then, when reading in each item, you construct an InventoryItem with the given properties, and return a single Vector<InventoryItem>.

If you're really attached to keeping track of all those individual Vectors, you could just return a Vector[] with all the vectors you have:

return new Vector[] { itemID, itemName, pOrdered, pInStore, pSold, manufPrice, sellingPrice };

Also, as Robin says, you should use the ArrayList container instead of Vector. The only thing that will change is that you need to change all calls to someVector.AddElement to someList.add.

Paul Fisher
I may use this, in a pinch, as I am still new to Java, and honestly, I just got out of a C++ class, and I am getting the two languages confused... Creating a class sounds like a good idea, thanks!
Danielle
In C++, you would probably use a "struct" to accomplish this. A struct is (basically) a class with all members defaulted to "public".
James Schek
A: 

Sounds like this should be tagged "Homework".

Okay, first of all, are you required to use all these Vectors, or is that your own decision? Though some may point out that using ArrayLists is better, I'd do away with them and create your own Item class.

This way, instead of having a conceptual item's properties distributed across multiple Vectors (the way you're doing now) you have 1 Item instance per item, with fields for all the data relevant to that item. Now, you only need one data structure (Vector or ArrayList) for all your item objects, and you can return that structure from getInventory().

InverseFalcon
Thank you, I didn't think to tag it homework... I'm still pretty new to this site.And, the reason that this is the way it is, is because the book I am working out of sucks, to be quite blunt. The book tells us to do it this way, but I think I'm going to be changing how I approach it, thanks!
Danielle
+1  A: 

You're doing a few things wrong.

Firstly, don't use Vector. Like, ever. If ordering is important to you, you want List on the API (and possibly ArrayList or LinkedList as an implementation).

Secondly, you're trying to have a large number of arrays have values that happen to line up. That's going to be nearly impossible to use. Just create a class that represents one record, and return the List of those.

Thirdly: do not catch that exception. You don't know what to do with it, and you're just going to confuse yourself. Only catch an exception if you have a really good idea what to do in the error case (printing out an error message without a stack is just about never the right thing).

The signature of your method is the most important part. If you get that right, the implementation doesn't matter nearly as much. Aim for something that looks like this:

List<Item> getInventory(File input) throws IOException {
}
Dustin
The exception was orginally placed because I couldn't get the file to load without it. I haven't had this problem before, but it gave me errors when I didn't use that try... catch block.I am going to try working with a class, like most of you have said, thank you!
Danielle
AFAIK, java is the first language to conduct the "checked exceptions" experiment. It ends up being mostly a failure because of cases like this. Much has been written on the subject: http://www.rockstarprogrammer.org/post/2007/jun/09/java-exception-antipatterns/
Dustin
A: 

The easiest way to declare the object would be something like

List<Vector<? extends Object>> inventoryItem = new ArrayList<Vector<? extends Object>>

but this has several problems, namely that Java's generics aren't reified, so you have to test and cast the contents of each vector that you get back. A better solution would be to define a container object that has each of the Vectors as fields and add to those.

However, this looks like it is really missing the point. Instead, you should define an InventoryItem who has each of your seven fields. Each time you read an object from the file, instantiate a new InventoryItem and populate its fields. Then, you add this to a single Vector.

Also, it is generally recommended that you do not use the Vector class. Instead, you should use ArrayList. Vector should really only be used if you need its synchronization properties, and even then you should consider wrapping some other list in a Collections.synchronizedList().

Finally, the places where you would want to catch just an Exception can be counted on one hand. You should really be catching an IOException and even that you might want to consider just rethrowing. Also, you should call printStackTrace() on the exception rather than System.out.println().

James
Thank you for the advice, this seems on par with what everyone is saying. I hope you all can forgive me newbness :) But, hey, I won't learn if I don't ask the questions, right? I will probably be creating a class. Thanks!
Danielle
A: 

While in general I heartily agree with the advice to use List/ArrayList instead of Vector, it is important to know why. Indeed, I have to vehemently disagree with Dustin who says not to use Vector "ever".

A Vector is in essence a synchronized ArrayList. If you truly need synchronization, by all means then, ignore Dustin's admonition, and use Vector.

There is another instance in which Vector is justified. And that is when you need to maintain compatibility with a pre-Java2 code base.

George Jempty
Thank you for the advice, I will keep that in mind. I do believe I have a lot of research to do now. Between my instructor being old school java, the book sucking, and all of this, I need to get this straightened out in my mind!
Danielle
George--that is incorrect. If you need a synchronized array list, best practice is "Collections.synchronizedList(new ArrayList())".
James Schek
If you truly need thread safety, using a class with every method synchronized does not give it to you. You have to think through all potential synchronization issues. Vector is only thread-safe in that it doesn't become corrupt. Collections.synchronizedList works for any List impl if that's enough
Dustin
Dustin--good point. This is evident by some of the collection and "primitive" types available through java.util.concurrent, such as Concurrent HashMap and the Atomic primitives.
James Schek
James: doesn't help you if you need backward compatability with a pre-Java2 codebase. Dustin: I said if you needed the ArrayList synchronized, I did *not* address thread safety in general.
George Jempty
The irony here is that I used these same arguments as team lead in my last contract *against* using Vector, they didn't like my answer there either. Don't judges say that, when neither side likes their decisions, they know it was right?
George Jempty
I don't know why this has been down-voted? Good advice IMO.
Richie_W
A: 

I find that a good rule of thumb is that it's never really a good idea to pass collections around outside your objects. They are obviously useful inside your object, but outside you lose control and they are not obvious.

Consider the principle of making your code readable instead of documenting it. If you take a collection, how does that tell the caller what to pass in? Even if you use generics, there is no way to assert control over what happens to the collection--someone could be adding to it and deleting from it in another thread after it's passed to you.

There is no reason not to create a business class that contains your collections along with the business logic to manipulate them (yeah, there is always business logic--it's the copy and paste code you'll find around the locations that you access the collection).

I used to find it frustrating that the JDK always seems to take arrays of built-in types rather than collections, but it makes a lot more sense after coming to terms with the idea that passing collections (like passing around any basic type) is just not a very good idea.

Bill K