From 9a70b44fd16a8beae9aa0c4d43f313c9163d67ea Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Thu, 2 Dec 2010 11:41:59 +0000 Subject: #6074 changes based on code review svn changeset:16273/svn branch:6.5 --- .../vaadin/data/util/AbstractBeanContainer.java | 124 ++++++++++----------- src/com/vaadin/data/util/BeanContainer.java | 42 ++++--- src/com/vaadin/data/util/BeanItemContainer.java | 49 ++++---- 3 files changed, 110 insertions(+), 105 deletions(-) (limited to 'src') diff --git a/src/com/vaadin/data/util/AbstractBeanContainer.java b/src/com/vaadin/data/util/AbstractBeanContainer.java index 0881cccf68..7adc57257a 100644 --- a/src/com/vaadin/data/util/AbstractBeanContainer.java +++ b/src/com/vaadin/data/util/AbstractBeanContainer.java @@ -55,13 +55,14 @@ import com.vaadin.data.Property.ValueChangeNotifier; * * @param * The type of the item identifier - * @param + * @param * The type of the Bean * * @since 6.5 */ -public abstract class AbstractBeanContainer implements Indexed, - Filterable, Sortable, ValueChangeListener, ItemSetChangeNotifier { +public abstract class AbstractBeanContainer implements + Indexed, Filterable, Sortable, ValueChangeListener, + ItemSetChangeNotifier { /** * Resolver that maps beans to their (item) identifiers, removing the need @@ -72,18 +73,19 @@ public abstract class AbstractBeanContainer implements Indexed, * has been set. * * @param - * @param + * @param * * @since 6.5 */ - public static interface BeanIdResolver extends Serializable { + public static interface BeanIdResolver extends + Serializable { /** * Return the item identifier for a bean. * * @param bean * @return */ - public IDTYPE getIdForBean(BT bean); + public IDTYPE getIdForBean(BEANTYPE bean); } /** @@ -93,7 +95,7 @@ public abstract class AbstractBeanContainer implements Indexed, * an object of type IDTYPE. */ protected class PropertyBasedBeanIdResolver implements - BeanIdResolver { + BeanIdResolver { private final Object propertyId; private transient Method getMethod; @@ -104,33 +106,22 @@ public abstract class AbstractBeanContainer implements Indexed, "Property identifier must not be null"); } this.propertyId = propertyId; - if (getGetter() == null) { - throw new IllegalArgumentException( - "Missing accessor for property " + propertyId); - } } - private Method getGetter() { + private Method getGetter() throws IllegalStateException { if (getMethod == null) { - try { - String propertyName = propertyId.toString(); - if (Character.isLowerCase(propertyName.charAt(0))) { - final char[] buf = propertyName.toCharArray(); - buf[0] = Character.toUpperCase(buf[0]); - propertyName = new String(buf); - } - - getMethod = getBeanType().getMethod("get" + propertyName, - new Class[] {}); - } catch (NoSuchMethodException ignored) { - throw new IllegalArgumentException(); + if (!model.containsKey(propertyId)) { + throw new IllegalStateException("Property " + propertyId + + " not found"); } + getMethod = model.get(propertyId).getReadMethod(); } return getMethod; } @SuppressWarnings("unchecked") - public IDTYPE getIdForBean(BT bean) throws IllegalArgumentException { + public IDTYPE getIdForBean(BEANTYPE bean) + throws IllegalArgumentException { try { return (IDTYPE) getGetter().invoke(bean); } catch (IllegalAccessException e) { @@ -149,7 +140,7 @@ public abstract class AbstractBeanContainer implements Indexed, * Methods that add a bean without specifying an ID must not be called if no * resolver has been set. */ - private BeanIdResolver beanIdResolver = null; + private BeanIdResolver beanIdResolver = null; /** * The item sorter which is used for sorting the container. @@ -178,12 +169,12 @@ public abstract class AbstractBeanContainer implements Indexed, * Maps all item ids in the container (including filtered) to their * corresponding BeanItem. */ - private final Map> itemIdToItem = new HashMap>(); + private final Map> itemIdToItem = new HashMap>(); /** * The type of the beans in the container. */ - private final Class type; + private final Class type; /** * A description of the properties found in beans of type {@link #type}. @@ -205,7 +196,7 @@ public abstract class AbstractBeanContainer implements Indexed, * @throws IllegalArgumentException * If {@code type} is null */ - protected AbstractBeanContainer(Class type) { + protected AbstractBeanContainer(Class type) { if (type == null) { throw new IllegalArgumentException( "The bean type passed to AbstractBeanContainer must not be null"); @@ -240,8 +231,8 @@ public abstract class AbstractBeanContainer implements Indexed, * @param bean * @return */ - protected BeanItem createBeanItem(BT bean) { - return new BeanItem(bean, model); + protected BeanItem createBeanItem(BEANTYPE bean) { + return new BeanItem(bean, model); } /** @@ -252,7 +243,7 @@ public abstract class AbstractBeanContainer implements Indexed, * * @return */ - public Class getBeanType() { + public Class getBeanType() { return type; } @@ -367,7 +358,7 @@ public abstract class AbstractBeanContainer implements Indexed, * * @see com.vaadin.data.Container#getItem(java.lang.Object) */ - public BeanItem getItem(Object itemId) { + public BeanItem getItem(Object itemId) { return itemIdToItem.get(itemId); } @@ -437,7 +428,7 @@ public abstract class AbstractBeanContainer implements Indexed, } // check if exactly the same items are there after filtering to avoid // unnecessary notifications - // this may be slow in some cases as it uses BT.equals() + // this may be slow in some cases as it uses BEANTYPE.equals() if (!originalItems.equals(filteredItemIds)) { fireItemSetChange(); } @@ -789,9 +780,14 @@ public abstract class AbstractBeanContainer implements Indexed, * @param bean * The bean to insert * - * @return true if the bean was added successfully, false otherwise + * @return BeanItem if the bean was added successfully, null + * otherwise */ - protected BeanItem internalAddAt(int position, IDTYPE itemId, BT bean) { + protected BeanItem internalAddAt(int position, IDTYPE itemId, + BEANTYPE bean) { + if (bean == null) { + return null; + } // Make sure that the item has not been added previously if (allItemIds.contains(bean)) { return null; @@ -804,7 +800,7 @@ public abstract class AbstractBeanContainer implements Indexed, // "filteredList" will be updated in filterAll() which should be invoked // by the caller after calling this method. allItemIds.add(position, itemId); - BeanItem beanItem = createBeanItem(bean); + BeanItem beanItem = createBeanItem(bean); itemIdToItem.put(itemId, beanItem); // add listeners to be able to update filtering on property @@ -837,9 +833,6 @@ public abstract class AbstractBeanContainer implements Indexed, * filters. *

* - * For internal use by subclasses only. This API is experimental and subject - * to change. - * * @param index * Internal index to add the new item. * @param newItemId @@ -848,9 +841,9 @@ public abstract class AbstractBeanContainer implements Indexed, * bean to be added * @return Returns new item or null if the operation fails. */ - private BeanItem addItemAtInternalIndex(int index, IDTYPE newItemId, - BT bean) { - BeanItem beanItem = internalAddAt(index, newItemId, bean); + private BeanItem addItemAtInternalIndex(int index, + IDTYPE newItemId, BEANTYPE bean) { + BeanItem beanItem = internalAddAt(index, newItemId, bean); if (beanItem != null) { filterAll(); } @@ -863,7 +856,7 @@ public abstract class AbstractBeanContainer implements Indexed, * * @see com.vaadin.data.Container#addItem(Object) */ - protected BeanItem addItem(IDTYPE itemId, BT bean) { + protected BeanItem addItem(IDTYPE itemId, BEANTYPE bean) { if (size() > 0) { // add immediately after last visible item int lastIndex = internalIndexOf(lastItemId()); @@ -878,8 +871,8 @@ public abstract class AbstractBeanContainer implements Indexed, * * @see com.vaadin.data.Container.Ordered#addItemAfter(Object, Object) */ - protected BeanItem addItemAfter(IDTYPE previousItemId, - IDTYPE newItemId, BT bean) { + protected BeanItem addItemAfter(IDTYPE previousItemId, + IDTYPE newItemId, BEANTYPE bean) { // only add if the previous item is visible if (previousItemId == null) { return addItemAtInternalIndex(0, newItemId, bean); @@ -905,7 +898,8 @@ public abstract class AbstractBeanContainer implements Indexed, * * @return Returns the new BeanItem or null if the operation fails. */ - protected BeanItem addItemAt(int index, IDTYPE newItemId, BT bean) { + protected BeanItem addItemAt(int index, IDTYPE newItemId, + BEANTYPE bean) { if (index < 0 || index > size()) { return null; } else if (index == 0) { @@ -927,14 +921,14 @@ public abstract class AbstractBeanContainer implements Indexed, * * @param bean * the bean to add - * @return BeanItem item added or null + * @return BeanItem item added or null * @throws IllegalStateException * if no bean identifier resolver has been set * @throws IllegalArgumentException - * if the resolved identifier for the bean is null + * if an identifier cannot be resolved for the bean */ - protected BeanItem addBean(BT bean) throws IllegalStateException, - IllegalArgumentException { + protected BeanItem addBean(BEANTYPE bean) + throws IllegalStateException, IllegalArgumentException { if (beanIdResolver == null) { throw new IllegalStateException( "Bean item identifier resolver is required."); @@ -963,14 +957,15 @@ public abstract class AbstractBeanContainer implements Indexed, * added, null to add to the beginning * @param bean * the bean to add - * @return BeanItem item added or null + * @return BeanItem item added or null * @throws IllegalStateException * if no bean identifier resolver has been set * @throws IllegalArgumentException - * if the resolved identifier for the bean is null + * if an identifier cannot be resolved for the bean */ - protected BeanItem addBeanAfter(IDTYPE previousItemId, BT bean) - throws IllegalStateException, IllegalArgumentException { + protected BeanItem addBeanAfter(IDTYPE previousItemId, + BEANTYPE bean) throws IllegalStateException, + IllegalArgumentException { if (beanIdResolver == null) { throw new IllegalStateException( "Bean item identifier resolver is required."); @@ -998,13 +993,13 @@ public abstract class AbstractBeanContainer implements Indexed, * the index (in the filtered view) at which to add the item * @param bean * the bean to add - * @return BeanItem item added or null + * @return BeanItem item added or null * @throws IllegalStateException * if no bean identifier resolver has been set * @throws IllegalArgumentException - * if the resolved identifier for the bean is null + * if an identifier cannot be resolved for the bean */ - protected BeanItem addBeanAt(int index, BT bean) + protected BeanItem addBeanAt(int index, BEANTYPE bean) throws IllegalStateException, IllegalArgumentException { if (beanIdResolver == null) { throw new IllegalStateException( @@ -1033,7 +1028,7 @@ public abstract class AbstractBeanContainer implements Indexed, * @throws IllegalStateException * if no bean identifier resolver has been set */ - protected void addAll(Collection collection) + protected void addAll(Collection collection) throws IllegalStateException { if (beanIdResolver == null) { throw new IllegalStateException( @@ -1041,7 +1036,7 @@ public abstract class AbstractBeanContainer implements Indexed, } int idx = internalIndexOf(lastItemId()) + 1; - for (BT bean : collection) { + for (BEANTYPE bean : collection) { IDTYPE itemId = beanIdResolver.getIdForBean(bean); if (internalAddAt(idx, itemId, bean) != null) { idx++; @@ -1065,7 +1060,8 @@ public abstract class AbstractBeanContainer implements Indexed, * @param beanIdResolver * to use or null to disable automatic id resolution */ - protected void setIdResolver(BeanIdResolver beanIdResolver) { + protected void setBeanIdResolver( + BeanIdResolver beanIdResolver) { this.beanIdResolver = beanIdResolver; } @@ -1074,7 +1070,7 @@ public abstract class AbstractBeanContainer implements Indexed, * * @return resolver used or null if automatic item id resolving is disabled */ - public BeanIdResolver getIdResolver() { + public BeanIdResolver getBeanIdResolver() { return beanIdResolver; } @@ -1082,10 +1078,10 @@ public abstract class AbstractBeanContainer implements Indexed, * Create an item identifier resolver using a named bean property. * * @param propertyId - * property identifier, which must map to a getter in BT + * property identifier, which must map to a getter in BEANTYPE * @return created resolver */ - protected BeanIdResolver createBeanPropertyResolver( + protected BeanIdResolver createBeanPropertyResolver( Object propertyId) { return new PropertyBasedBeanIdResolver(propertyId); } diff --git a/src/com/vaadin/data/util/BeanContainer.java b/src/com/vaadin/data/util/BeanContainer.java index dbfc197674..3fe7ae20ca 100644 --- a/src/com/vaadin/data/util/BeanContainer.java +++ b/src/com/vaadin/data/util/BeanContainer.java @@ -34,7 +34,7 @@ import com.vaadin.data.Item; * * @param * The type of the item identifier - * @param + * @param * The type of the Bean * * @see AbstractBeanContainer @@ -42,10 +42,10 @@ import com.vaadin.data.Item; * * @since 6.5 */ -public class BeanContainer extends - AbstractBeanContainer { +public class BeanContainer extends + AbstractBeanContainer { - public BeanContainer(Class type) { + public BeanContainer(Class type) { super(type); } @@ -69,7 +69,7 @@ public class BeanContainer extends * @see com.vaadin.data.Container#addItem(Object) */ @Override - public BeanItem addItem(IDTYPE itemId, BT bean) { + public BeanItem addItem(IDTYPE itemId, BEANTYPE bean) { if (itemId != null && bean != null) { return super.addItem(itemId, bean); } else { @@ -78,13 +78,13 @@ public class BeanContainer extends } /** - * Adds the bean after the given bean. + * Adds the bean after the given item id. * * @see com.vaadin.data.Container.Ordered#addItemAfter(Object, Object) */ @Override - public BeanItem addItemAfter(IDTYPE previousItemId, IDTYPE newItemId, - BT bean) { + public BeanItem addItemAfter(IDTYPE previousItemId, + IDTYPE newItemId, BEANTYPE bean) { if (newItemId != null && bean != null) { return super.addItemAfter(previousItemId, newItemId, bean); } else { @@ -107,7 +107,8 @@ public class BeanContainer extends * @return Returns the new BeanItem or null if the operation fails. */ @Override - public BeanItem addItemAt(int index, IDTYPE newItemId, BT bean) { + public BeanItem addItemAt(int index, IDTYPE newItemId, + BEANTYPE bean) { if (newItemId != null && bean != null) { return super.addItemAt(index, newItemId, bean); } else { @@ -124,35 +125,42 @@ public class BeanContainer extends * @param propertyId * the identifier of the property to use to find item identifiers */ + // overridden to make public public void setBeanIdProperty(Object propertyId) { - setIdResolver(createBeanPropertyResolver(propertyId)); + setBeanIdResolver(createBeanPropertyResolver(propertyId)); } @Override - public void setIdResolver(BeanIdResolver beanIdResolver) { - super.setIdResolver(beanIdResolver); + // overridden to make public + public void setBeanIdResolver( + BeanIdResolver beanIdResolver) { + super.setBeanIdResolver(beanIdResolver); } @Override - public BeanItem addBean(BT bean) throws IllegalStateException, - IllegalArgumentException { + // overridden to make public + public BeanItem addBean(BEANTYPE bean) + throws IllegalStateException, IllegalArgumentException { return super.addBean(bean); } @Override - public BeanItem addBeanAfter(IDTYPE previousItemId, BT bean) + // overridden to make public + public BeanItem addBeanAfter(IDTYPE previousItemId, BEANTYPE bean) throws IllegalStateException, IllegalArgumentException { return super.addBeanAfter(previousItemId, bean); } @Override - public BeanItem addBeanAt(int index, BT bean) + // overridden to make public + public BeanItem addBeanAt(int index, BEANTYPE bean) throws IllegalStateException, IllegalArgumentException { return super.addBeanAt(index, bean); } @Override - public void addAll(Collection collection) + // overridden to make public + public void addAll(Collection collection) throws IllegalStateException { super.addAll(collection); } diff --git a/src/com/vaadin/data/util/BeanItemContainer.java b/src/com/vaadin/data/util/BeanItemContainer.java index 1ce6821e07..a9c8a0a62c 100644 --- a/src/com/vaadin/data/util/BeanItemContainer.java +++ b/src/com/vaadin/data/util/BeanItemContainer.java @@ -28,13 +28,14 @@ import java.util.Collection; * bean properties are not supported. *

* - * @param + * @param * The type of the Bean * * @since 5.4 */ @SuppressWarnings("serial") -public class BeanItemContainer extends AbstractBeanContainer { +public class BeanItemContainer extends + AbstractBeanContainer { /** * Bean identity resolver that returns the bean itself as its item @@ -65,10 +66,10 @@ public class BeanItemContainer extends AbstractBeanContainer { * @throws IllegalArgumentException * If {@code type} is null */ - public BeanItemContainer(Class type) + public BeanItemContainer(Class type) throws IllegalArgumentException { super(type); - super.setIdResolver(new IdentityBeanIdResolver()); + super.setBeanIdResolver(new IdentityBeanIdResolver()); } /** @@ -93,14 +94,12 @@ public class BeanItemContainer extends AbstractBeanContainer { */ @SuppressWarnings("unchecked") @Deprecated - public BeanItemContainer(Collection collection) + public BeanItemContainer(Collection collection) throws IllegalArgumentException { // must assume the class is BT // the class information is erased by the compiler - super((Class) getBeanClassForCollection(collection)); - super.setIdResolver(new IdentityBeanIdResolver()); - - addAll(collection); + this((Class) getBeanClassForCollection(collection), + collection); } /** @@ -134,11 +133,11 @@ public class BeanItemContainer extends AbstractBeanContainer { * @throws IllegalArgumentException * If {@code type} is null */ - public BeanItemContainer(Class type, - Collection collection) + public BeanItemContainer(Class type, + Collection collection) throws IllegalArgumentException { super(type); - super.setIdResolver(new IdentityBeanIdResolver()); + super.setBeanIdResolver(new IdentityBeanIdResolver()); if (collection != null) { addAll(collection); @@ -172,7 +171,7 @@ public class BeanItemContainer extends AbstractBeanContainer { * The collection of beans to add. Must not be null. */ @Override - public void addAll(Collection collection) { + public void addAll(Collection collection) { super.addAll(collection); } @@ -189,9 +188,10 @@ public class BeanItemContainer extends AbstractBeanContainer { * @see com.vaadin.data.Container.Ordered#addItemAfter(Object, Object) */ @SuppressWarnings("unchecked") - public BeanItem addItemAfter(Object previousItemId, Object newItemId) - throws IllegalArgumentException { - return super.addBeanAfter((BT) previousItemId, (BT) newItemId); + public BeanItem addItemAfter(Object previousItemId, + Object newItemId) throws IllegalArgumentException { + return super.addBeanAfter((BEANTYPE) previousItemId, + (BEANTYPE) newItemId); } /** @@ -206,9 +206,9 @@ public class BeanItemContainer extends AbstractBeanContainer { * @return Returns the new BeanItem or null if the operation fails. */ @SuppressWarnings("unchecked") - public BeanItem addItemAt(int index, Object newItemId) + public BeanItem addItemAt(int index, Object newItemId) throws IllegalArgumentException { - return super.addBeanAt(index, (BT) newItemId); + return super.addBeanAt(index, (BEANTYPE) newItemId); } /** @@ -219,8 +219,8 @@ public class BeanItemContainer extends AbstractBeanContainer { * @see com.vaadin.data.Container#addItem(Object) */ @SuppressWarnings("unchecked") - public BeanItem addItem(Object itemId) { - return super.addBean((BT) itemId); + public BeanItem addItem(Object itemId) { + return super.addBean((BEANTYPE) itemId); } /** @@ -231,7 +231,7 @@ public class BeanItemContainer extends AbstractBeanContainer { * @see com.vaadin.data.Container#addItem(Object) */ @Override - public BeanItem addBean(BT bean) { + public BeanItem addBean(BEANTYPE bean) { return addItem(bean); } @@ -239,10 +239,11 @@ public class BeanItemContainer extends AbstractBeanContainer { * Unsupported in BeanItemContainer. */ @Override - protected void setIdResolver( - AbstractBeanContainer.BeanIdResolver beanIdResolver) + protected void setBeanIdResolver( + AbstractBeanContainer.BeanIdResolver beanIdResolver) throws UnsupportedOperationException { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException( + "BeanItemContainer always uses an IdentityBeanIdResolver"); } } -- cgit v1.2.3