• Post Reply Bookmark Topic Watch Topic
  • New Topic

Generics. My own Pool class. Why is this not ok?  RSS feed

 
Neil Cartmell
Ranch Hand
Posts: 150
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello.

I wanted to make a very simple Pool class to keep a pool for any Object. I thought I had an idea that worked well then I realized I would have to cast the returned object, so I thought I would try and make it with Generics.

I don't understand why there is an error with this code. The problem is in line 7 in class Pool. This is the error I get in Netbeans

no suitable method found for add(usefulstuff.Poolable)
method java.util.ArrayList.add(int,T) is not applicable
(actual and formal argument lists differ in length)
method java.util.ArrayList.add(T) is not applicable
(actual argument usefulstuff.Poolable cannot be converted to T by method invocation conversion

So in class Pool, this is what I thought would be happening. I create the class passing an object that extends Poolable. In the contructor I pass it a referrence to an object that is type T, which is stated in the class name as extending interface Poolable. The method create() listed in Poolable returns a Poolable object. The line Poolable p = t.create() does not get an error. The ArrayList is passed type T. So if it is an ArrayList of Poolable objects why won't it let me add a Poolable object?



 
Matthew Brown
Bartender
Posts: 4568
9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
T can be any type that implements Poolable. So let's look at what that class looks like if you create it with T = ConcretePoolable (where ConcretePoolable implements Poolable):

On line 7 you're trying to add a Poolable to an ArrayList<ConcretePoolable>. The compiler doesn't know if this is safe - it could be some other implementation of Poolable.

There are a few changes you could make that would make this compile, but I'm not quite sure what the intent it, so I'm not sure which one to suggest.

(Oh, you probably want to initialise pool somewhere as well )
 
Neil Cartmell
Ranch Hand
Posts: 150
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Mathew. I understand. I just had to change the ArrayList to ArrayList<Poolable>. Thanks again.



 
Neil Cartmell
Ranch Hand
Posts: 150
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
OK I'm stuck again. This compiles but I'm back to square one of having to cast the returned object. The get method has to return a Poolable. But what I want is for it return the concrete object that created the class with AND to have an arrayList that stores them all. And call the methods of the Poolable interface. Is this possible? I definitely use the word pool too much in this code though don't I? lol

This class is supposed to be so I can keep a pool of any object that implements Poolable so I can get the object without having to cast it.

 
Neil Cartmell
Ranch Hand
Posts: 150
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
OK I think I may have sorted it out. I'm yet to test it but it gets no errors.

 
Wouter Oet
Bartender
Posts: 2700
IntelliJ IDE Opera
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
3 comments:

The field pool can be of type List<T> so that you can program against against an interface.

Your pool can only be used by classes which have a default constructor otherwise the createInstance method will return null.

Your pool can't be used by multiple threads.
 
Neil Cartmell
Ranch Hand
Posts: 150
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Wouter Oet wrote:The field pool can be of type List<T> so that you can program against against an interface.


Ok I'll change this.

Wouter Oet wrote:Your pool can't be used by multiple threads.


If I change the getMethod to be synchronized, will that be enough to make it thread safe? Threads are something I'm still trying to get my head around. But as far as I understandl if that get() method is made synchronized, then only one thread can manipulate the pool at a time.

Wouter Oet wrote:Your pool can only be used by classes which have a default constructor otherwise the createInstance method will return null.


Now this is what has been giving my the problem. I thought maybe I could make each object that implements poolable have a create() method that knows how to create it's self. But as far as I can tell that would have to return an object of type Poolable, which means I would have to cast it, which means it's not safe. Right?

Is this actually possible to do. I'm looking around the internet now and as far as I can see, it isn't. Or it's extreamly complicated.

Edit: I have now sorted out the bugs in my code which the below paragraph talks about.

At the moment, although flawed, it does kind of work how I want, but it's still giving me problems. The reason I wanted a pool class is because I have a class called ArrayLocation that has various information about the cell index in an array of arrays. It's been a very useful class to me. With a lot of the programs I'm writing though it seems way more convient to create a new ArrayLocation everytime I need one. Like for example when the player moves the mouse, and then it is used in another class, that is not on the event dispatch thread for it's calculations. So I thought a pool would be great. But all it has done is introduced hard to detect bugs caused by the fact it's very hard to know when it's ok to make that ArrayLocation available again in the pool.

I'm starting to think it would be better to constantly create objects than have very confusing code. Although I'm sure there will be other times when it's use can be much more simple then what I'm doing now.

 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Wouter Oet wrote:The field pool can be of type List<T> so that you can program against against an interface.

Actually, I think it might be better to make it a Set<T>, since I presume you don't need (or want) to keep duplicates.

@Neil: While I don't generally advocate looking at performance, in this case you might want to, since your pool is actually being used as an object cache; and caches are something where performance is quite critical. Right now, you have to search through your entire list in order to determine whether the object is available, but there are several existing collections (such as a HashSet) that can do that search much quicker (in fact, the easiest may be an IdentityHashMap where the key and the value are the same).

Alternatively, you could look at LinkedHashMap; it has a method called removeEldestEntry() that you can use to keep your "pool size" constant and implement a 'least recently used' policy, which is often very useful for pools.

But maybe that's for later. Get a nice simple pool working first.

Winston
 
Neil Cartmell
Ranch Hand
Posts: 150
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote:
Actually, I think it might be better to make it a Set<T>, since I presume you don't need (or want) to keep duplicates.

@Neil: While I don't generally advocate looking at performance, in this case you might want to, since your pool is actually being used as an object cache; and caches are something where performance is quite critical. Right now, you have to search through your entire list in order to determine whether the object is available, but there are several existing collections (such as a HashSet) that can do that search much quicker (in fact, the easiest may be an IdentityHashMap where the key and the value are the same).

Alternatively, you could look at LinkedHashMap; it has a method called removeEldestEntry() that you can use to keep your "pool size" constant and implement a 'least recently used' policy, which is often very useful for pools.

But maybe that's for later. Get a nice simple pool working first.

Winston


Thanks you are right. I did think it seemed a bit rubbish to have to loop through the array everytime. I will make the changes you suggest.

I've used it in my program and it works great. The only draw back is that if the object I try to keep a pool for doesn't have a default constructor then it won't work propperly. But if there is no simple way to rectify this i'm going to let it slide. This class is just for me after all and I wanted to keep it very simple and I know it has to have a default so it will work fine for me.

I used a static variable creationCount in the constructor of my ArrayLocation class. Before i was creating hundreds in a matter of seconds, but now it's never created more than 3.

ps. Was I right to make the get() method synchronized?
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Neil Cartmell wrote:ps. Was I right to make the get() method synchronized?

I don't know if there's any "right" or "wrong" about it, but most designers treat synchronization (or Thread-safety) as something separate from the actual behaviour of a class. The nice thing about that is that you can deal with it when you've got your "basic model" working and it doesn't distract you from working out what the class is supposed to do.

A lot of the original collection classes (eg, Stack, Vector and Hashtable) were written from scratch to be Thread-safe, but when the Java Collections Framework was introduced that policy was totally reversed. Now, the new classes have no synchronization in them at all; and if you need a synchronized alternative you use one of the views provided by the Collections class.

My advice: Do the same. Write your class and get it working in a single-threaded environment. Then, if you think you need it, provide a second, synchronized, wrapper class.

Winston
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!