5

I'm having a bit of a problem grasping something - I might be going about this completely wrong.

I am trying to create a class which extends ArrayList but has several methods which increase the functionality (at least for the program I am developing.)

One of the methods is a findById(int id), which searches each ArrayList object for a particular id match. So far it's working, but it won't let me do for (Item i : this) { i.getId(); }

I don't understand why?

Full code:

public class CustomArrayList<Item> extends ArrayList<Item> {

    // declare singleton instance
    protected static CustomArrayList instance;

    // private constructor
    private CustomArrayList(){
        // do nothing
    }

    // get instance of class - singleton
    public static CustomArrayList getInstance(){
        if (instance == null){
            instance = new CustomArrayList();
        }
        return instance;
    }

    public Item findById(int id){
        Item item = null;
        for (Item i : this) {
            if (i.getId() == id) {
                      // something
         }
        }
        return item;
    }
    public void printList(){
        String print = "";
        for (Item i : this) {
            print += i.toString() + "\n";
        }
        System.out.println(print);
    }
}
Cody
  • 8,686
  • 18
  • 71
  • 126

2 Answers2

7

Change

public class CustomArrayList<Item> extends ArrayList<Item> {

to

public class CustomArrayList extends ArrayList<Item> {

I suspect Item is the name of the class that you want to store in the list. By adding <Item> after CustomArrayList you're introducing a type parameter which shadows this class.


With the <Item> parameter, your code is equal to

public class CustomArrayList<T> extends ArrayList<T> {
    // ...
        for (T i : this) { i.getId(); }
    // ...
}

which obviously won't always work, as T may refer to any type.

aioobe
  • 413,195
  • 112
  • 811
  • 826
  • Awesome. Is that just the proper syntax, or was I trying to do something else? Thank you so much! Quick reply. – Cody Jun 29 '11 at 17:23
  • 1
    That is probably the proper syntax for your scenario. Updated the answer to explain why. – aioobe Jun 29 '11 at 17:25
  • 1
    Thank you! That explains it very well. – Cody Jun 29 '11 at 17:27
  • To add to this, if you want to support various element types with common super type `Item`, use `public class ItemArrayList extends ArrayList { ... }`. – Dilum Ranatunga Jun 29 '11 at 17:33
2

What is getId()? Presumably it's a method in some class, but we don't know which class.

If you've actually got a class called Item with a getId() method, which this is meant to be a list of, you simply need to stop your class from being generic. So instead of this:

public class CustomArrayList<Item> extends ArrayList<Item> {

you want:

public class CustomArrayList extends ArrayList<Item> {

Currently within your class, Item doesn't refer to a class called Item, it refers to a type parameter called Item.

Now personally:

  • I wouldn't avoid creating singletons unless you really have to
  • If you have to, I'd avoid creating them in the way you have (which isn't thread-safe)
  • I wouldn't extend ArrayList<> unless I really had to, preferring composition over inheritance
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • I'm creating a singleton for the reason that I can only have one. I know that the way I'm creating it isn't thread safe - I would need to have a "lock method", correct? What aioobe posted solved my problem, but I'm still interested in creating a thread-safe singleton. – Cody Jun 29 '11 at 17:31
  • 1
    The preferred way to create a singleton in Java 1.5+ is to use an Enum. See for instance this question: http://stackoverflow.com/questions/70689/efficient-way-to-implement-singleton-pattern-in-java – aioobe Jun 29 '11 at 17:42