SelectItemIndexConverter (contribution)

[GoogleCodeExporter]

In my company we're currently replacing our own JSF utility library with 
Omnifaces so we'd like to contribute some stuff to the community.

We created a simple converter which allows to populate any (e.g Pojo/DTO/Model) 
objects as value of <f:selectItems> and have JSF convert those automatically 
back without the need to provide a custom converter which may need to do the 
job based on possibly expensive service/DAO operations. A object is identified 
by it's absolute index position within the selectItems list. Using the 
SelectItemIndexConverter you don't need to rely on the toString() method of 
your object nether do you have to create another converter for your object. It 
is also useful for object's which don't have a unique property (value objects) 
or you don't want to expose the object's unique key.

Here is the file:
http://code.google.com/r/patrickdobler-mimacom-contributions2/source/browse/src/
org/omnifaces/converter/SelectItemsIndexConverter.java

Showcase Example:
http://code.google.com/r/patrickdobler-omnifaces-showcase-contributions/source/b
rowse/WebContent/showcase/converters/SelectItemsIndexConverter.xhtml

Original issue reported on code.google.com by patrick....@gmail.com on 19 Nov 2012 at 10:54

[GoogleCodeExporter]

Thank you for your contribution. We will consider it.

It should however be documented that this way the "Validation Error: Value is 
not valid" will never occur (which should signal the developer that the list of 
items has incompatibly changed and/or that the user is potentially submitting 
an item the user isn't supposed to submit) and thus the developer needs to 
ensure that the provided selectitems list is exactly the same during the 
postback request as it was during initial request (which can be achieved by 
performing initialization in (post)constructor of a view or broader scoped 
bean).

Original comment by balusc on 20 Nov 2012 at 8:47

[GoogleCodeExporter]

Patrick, the code looks nice indeed.

This method in fact is in a way what the default DataTable is doing; it matches 
the row a user e.g. clicked on based on the index of the collection that was 
used to render the table. This as opposed to components using SelectItems that 
effectively match on a key.

We have been arguing that both these two should be able to use both index 
matching and key matching, so your proposal falls right into place there ;)

As Bauke indicates, the index has the disadvantage that if the collection of 
SelectItems used for rendering is not exactly the same as the one that is 
available after post-back, the converter will silently select the wrong object. 
There's a validation step being done by most components, e.g. in 
UISelectMany#validateValue, but this will only validate that the converted 
object is present in the collection of SelectItems (which is thus by definition 
true, since it was just taken from there).

The advantages are exactly the ones you mentioned. No need to rely on or 
implement toString(), no need to build your own converter and indeed no need to 
expose the key if that's a problem.

So as long as we can make it clear to the user what the tradeoffs are with both 
approaches, I think your proposed converter will be a great addition.

A few very small things about the implementation:

The following line in getAsString is relatively expensive:

final List<SelectItem> selectItemList = 
SelectItemsCollector.collectFromParent(context, component);

I used the same one in getAsObject, but in that case it will be called only 
once for a post-back. In the case of getAsString it will be called over and 
over again for each item that's rendered. Maybe we'd better cache this in 
FacesContext#getAttributes for the given component.

A very small remark about code convention in the following line:

SelectItem aSelectItem = selectItemList.get(i);

In OmniFaces we use the convention of just writing the variable name as the 
class name, without the 'a' prefix. E.g.

SelectItem selectItem = selectItemList.get(i);

As for the methods bothEquals and botNull, perhaps they'd be more at home in 
the Utils class.

Original comment by arjan.ti...@m4n.nl on 20 Nov 2012 at 10:44

[GoogleCodeExporter]

Committed: 
http://code.google.com/p/omnifaces/source/detail?r=a8f9251f611ebe8271e7c82541d77
027560ed7ff

I've made some improvements, the major one was that this converter is now also 
compatible with SelectItemGroup by using 
SelectItemUtils#collectAllValuesFromSelectItems() instead to collect all 
available values, also those nested in groups. The 
SelectItemsCollector#collectFromParent() only returns the immediate items, not 
the nested values of the select item groups. The collected values are stored as 
a context attribute.

Thank you, Patrick :)

Original comment by balusc on 23 Nov 2012 at 1:44

• Changed state: Fixed

[GoogleCodeExporter]

BTW: bothEquals() and bothNull() were unnecessary.

Original comment by balusc on 23 Nov 2012 at 1:46

[GoogleCodeExporter]

Hi guys,

Thank you very much for your constructive feedback.

1. I do agree with the fact that using the SelectItemsIndexConverter the 
developer has to make sure that the exactly same list is preserved on a 
postback. In our architecture we usually bind the list of values to a 
ViewScoped model (or broader) and initialize that list in a corresponding 
controller invoked by the preRenderView event if it was NOT a postback... That 
is why we never had that postback issue

2. Caching of these values seems legit 

Looking forward to use omniface's SelectItemsIndexConverter and test it against 
our various use-cases. As far as I can tell from looking at your version of the 
code, everything looks fine. 

Cool! =)

Original comment by patrick....@gmail.com on 23 Nov 2012 at 8:25

[GoogleCodeExporter]

It's available in the attached snapshot of today.

Original comment by balusc on 23 Nov 2012 at 1:05

Attachments:

• omnifaces-1.3-SNAPSHOT-20121123.jar