Vaadin 6.6.6. BeanItemContainer is evil, seriously

My BeanItemContainers were happily functioning like:

new BeanItemContainer<Frequency>(list))

but now it sais it’s deprecated and I should write it like:

new BeanItemContainer(Frequency.class,
										list))

but the container itself uses generics and Eclipse marks it as untyped, which means I have to write it like:

new BeanItemContainer<Frequency>(Frequency.class,
										list)

which doesn’t look right at all.

And I only have about 30 such entries in a file that makes Eclipse stutter while I’m editing the file.

What was wrong with the generic way in the first place that now it requires the class as a parameter?

This did not come in 6.6.6… and the repetition is needed because of limitations of Java generics.

The actual Class object is needed to find the available properties, to perform some checks etc. However, Java generics are (unfortunately) a compile time only mechanism and the information about generics only exists as a “description” in the class file.

The old version performed something very unstable and evil: it looked at the first element in the list to find the class, and assumed all elements share that class. This could have very nasty side effects when using multiple subclasses of a class in the container, and empty lists also caused problems.

Hi,

unlike you, I welcome changes made to BeanItemContainer. The former version was lacking and obstructive in comparison tbh.

Major: It wasn’t possible to instantiate BeanItemContainer upon empty collection as the BeanItemContainer internally picked up 1st element from collection to guess item type. With a new version this limitation no longer exist.

The item type is now being specified via extra argument for a reason - known workaround for Java’s inability to induce Class type from Generics type, which effectively prevents from instantiating or introspecting generics type itself (this is what BeanItemContainer does). Hence the class type has to be passed explicitely in a separate argument. Even though it may look weird and unnecessary at first glance, it’s pretty common practise.

Well that’s at least my understanding.

EDIT: beaten by Henri.

Tomas

BTW, the ability to say T.class to get the actual class of a generic is one of the planned changes to Java.

There are some evil tricks using reflection to get the same effect, but the best solution to avoid passing the .class in the current version relies on instantiation as if creating an instance of an anomymous inner class. See
this article
for examples.

This said, most people just pinch their noses and pass the class.

Good article.

I don’t have the insight about where this is going.

I don’t extend my POJOs because they’re storage, which also means the compiler will inline the accessors/mutators if made final I believe.

Even in some complex architecture where extending POJOs may have meaning to represent a entity inheritance there would be no problem because polymorphism gets the right instance.

(sorry about the edit Jean-François Lamy :slight_smile: I have to pass the class in probably 100 different places (and edit in the right class manually), and I suspect this isn’t the final solution (it’s probably going to change after Oracle will bork Java) , while the number of places where I’ll be using this is also bound to increase.

In my use case:

globalTable
								.setContainerDataSource(new BeanItemContainer<Frequency>(
										Frequency.class, null));

If the collection is null, I don’t need to instantiate a BeanItemContainer, which means I should be able to pass null to globalTable.setContainerDataSource(null), and get an empty table instead of an error about ids, and with this said:

create a BeanItemContainer factory that takes a collection, and:
if collection is null or empty returns null instead of a container (because globalTable should know to handle nulls),
if its not null get the class either by passing it as a parameter to the factory and pass it along to the BeanItemContainer (if we have to - I don’t see why)
(
Actually I think here we should let polymorphism do it’s thing and use Object.class (didn’t go further to see where its going, but if it’s sending Object.class, it looks good for code removal) :

public MyBeanItemContainer(Class<? super BEANTYPE> type,
            Collection<? extends BEANTYPE> collection)
            throws IllegalArgumentException {
        super(Object.class);

all classes are objects.
)
This is also safe because using generics with the List, it won’t let you put in the list an Integer and a String for example
or
throw an exception from BeanItemContainer constructor if the collection is null, and set the BeanItemContainer reference to null (which doesn’t seem ideal to me)

import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

public class CollectionsTest {

	@SuppressWarnings("unused")
	public static void main(String[] args) {
		List<Duck> list = new ArrayList<Duck>();
		list.add(new Duck());
		list.add(new Duckie());
		list.add(new Duckhee());
		DuckHerder<Duck> dh = new DuckHerder<Duck>(list);
		dh.herdDucks();
		dh.herdDucksSimpler();

		// ideal (maybe)
		// ContainersAbstractFactory(ContainerType.BEAN)
		// return BeanItemContainerFactory.create(collection)
		// returns
		// 		if collectioin is null or empty
		// 			null (data source clients should not freak out)
		// 		else
		// 			populated BeanItemContainer (refering to it with the Container
		// interface or abstract class)

		// not ideal
		DuckHerder3 duckHerder3;
		try {
			new DuckHerder3();
		} catch (Exception e) {
			duckHerder3 = null;
		}
		// pass duckHerder3
	}

}

/**
 * what is the point of initializing a container that's not going to hold
 * anything? better off not build the container at all
 */
class DuckHerder<T> {
	Collection<? extends T> list;

	@SuppressWarnings("unchecked")
	DuckHerder(Collection<? extends T> list) {
		this.list = list;
	}

	public void herdDucks() {

		java.lang.Class<? extends T> c = (java.lang.Class<? extends T>) (list
				.iterator().hasNext() ? list.iterator().next().getClass()
				: null);

		for (T t : list) {
			Object o = null;

			if (c != null)
				o = c.cast(t);
			else
				break;
			Method[] allMethods = o.getClass().getDeclaredMethods();
			for (Method m : allMethods)
				if (m.getName().startsWith("quack"))
					try {
						m.invoke(o, null);
					} catch (Exception e) {
						e.printStackTrace();
					}
		}
	}

	public void herdDucksSimpler() {

		for (T t : list) {
			Method[] allMethods = t.getClass().getDeclaredMethods();
			for (Method m : allMethods)
				if (m.getName().startsWith("quack"))
					try {
						m.invoke(t, null);
					} catch (Exception e) {
						e.printStackTrace();
					}
		}
	}

}

class DuckHerder3 {

	public DuckHerder3() throws Exception {
		throw new NullPointerException();
	}
}

class Duck {

	public void quack() {
		System.out.println("Quack");
	}
}

class Duckie extends Duck {

	public void quack() {
		System.out.println("Quackie");
	}
}

class Duckhee extends Duckie {

	public void quack() {
		System.out.println("Quackhee");
	}
}

Passing the class as a parameter would’ve sufficed as we can put in a @SupressWarning(‘unchecked’) in Eclipse.
My reason is that I can edit all the old spots with a single regex perhaps

I still think that tables should know how to handle incoming nulls, as in display an empty table, this way, the programmer has two choices after checking for null from the BeanContainer for instance:

  • pass the null to the table expecting (or not) an empty table, which speaks for itself
  • do not even create the table
    In JSP this was very straightforward

Found an interesting article too after I run into a generic arrays issue trying to piggyback two lists on a single return:

http://code.stephenmorley.org/articles/java-generics-type-erasure/

also here’s an answer (not the selected one but the one below):

http://stackoverflow.com/questions/217065/cannot-create-an-array-of-linkedlists-in-java