myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Martin Marinschek" <martin.marinsc...@gmail.com>
Subject Re: [All MyFaces] Local properties vs. ValueExpression/Binding
Date Wed, 09 Apr 2008 02:14:12 GMT
I would slightly lean to this solution:

Solution two:

Check if EL is set and ValueExpression.isReadOnly(ELContext context)
returns false
 set EL
else set local

and fix the el-implementation if it doesn't do what it is expected to do.

Second problem (resetting) is nothing we can do generally; we'd have
to do it case-by-case.

regards,

Martin

On Mon, Mar 24, 2008 at 7:04 PM, Andrew Robinson
<andrew.rw.robinson@gmail.com> wrote:
> The bug https://issues.apache.org/jira/browse/TOMAHAWK-858 has made me
>  want to bring this up for discussion of the entire team. I really
>  don't like the solution they are posing as it will cause backward
>  compatibility problems and is not a full solution. This problem is not
>  unique to the Tomahawk tabbed pane, so I would love to see a MyFaces
>  wide (and maybe a JSF 2.0) fix (or approach) to the problem.
>
>  The issue is that several components use a "setXxx(...)" method to
>  update components. This typically is done in renderer code. Here is a
>  short list of components that I know are affected:
>
>  Tomahawk data scroller / data table and the first attribute
>  Tomahawk tabbed pane
>  Trinidad UIXShowDetail (I checked in a one component fix for this one,
>  but I am not 100% happy with it)
>
>  I am sure there are more. The problem is that if I have this code (I
>  am picking on the data table it is the easiest example):
>
>  <t:dataTable first="#{bean.firstRowIndex}"...>
>   <t:dataScroller ...>
>  ...
>
>  The data scroller has the code UIData.setFirst(...) when the event is broadcast.
>
>  Typically all MyFaces getter code is written as:
>
>  if there is a local value
>   return it
>  if not, get the value binding / value expression
>   if set, evaluate it
>  if the value is null or there was no EL, then return a default value
>  if available, or else null
>
>  The setter code is:
>
>  set the local value
>
>  Even with Trinidad using a FacesBean, it still suffers from the local
>  vs. EL problem. What ends up being the major issue is that the local
>  value always takes precedence over the EL value. This is very rarely
>  the desired behavior. The local value is only really useful when
>  component binding is being used and the page author is not using EL to
>  set attributes.
>
>  The solution is more complex. In issue TOMAHAWK-858, someone has
>  proposed to use EL if it is set, but they did not examine the problem
>  domain. For example:
>
>  <t:dataTable first="#{bean.condition ? 0 : bean.firstRowIndex}" ...
>
>  In this crude example, perhaps the hard-coded 0 is to reset the table
>  to the first record on another link (please do not over analyze the
>  example, I know it is far from perfect). The problem with this is that
>  in using a conditional EL expression, this is no longer set-able! So
>  if the data scroller code attempts to assign 20 (the next page lets
>  say) to the first EL, it will throw an exception, because a
>  conditional statement cannot be set.
>
>  One solution to this is:
>  if EL is set,
>  try to set the EL
>  if that throws an exception, set the local value
>
>  This would technically work, but I hate the code. It is unpredictable
>  and bad for performance (exceptions are expensive and should be
>  avoided).
>
>  Solution two:
>
>  Check if EL is set and ValueExpression.isReadOnly(ELContext context)
>  returns false
>   set EL
>  else set local
>
>  Problem is that I do not think that this is always accurate.
>
>  The other problem is that once the local value is set, it cannot
>  always be cleared:
>
>  private Integer _first;
>  public int getFirst() {
>   if (_first != null) return _first.intValue();
>   ValueBinding ...
>  }
>  public int setFirst(int first) { _first = new Integer(first); }
>
>  Here, there is no way to set _first back to null. We could change the
>  APIs drastically of Tomahawk and Trinidad so that the generation
>  plugin always has to use objects and not primitives, but that would
>  break a lot of code and is not a nice API when null is never returned
>  for a getter when a default value is used.
>
>  Another option is a case-by-case fix where the property can be made
>  "transient". Meaning that the set method does nothing and the get
>  method is always used. This is a partial fix, but is ugly and requires
>  that the component users always update the values behind the EL
>  manually based on events
>
>  If EL allowed for different get & set that may work better:
>
>  <t:dataTable first="#{get: bean.condition ? 0 : bean.firstRowIndex,
>  set: bean.firstRowIndex}" ...
>
>  In this case the value expression would evaluate different expressions
>  for get vs. set. The problem here is that it is harder to write,
>  understand and would require an EL change that should be part of the
>  J2EE spec. which would be ugly.
>
>  Another option is for the user to be able to specify which values
>  should be local and which should set via EL, but I don't see a clean
>  way of doing this without making an ugly API (way to many
>  attribute-metadata attributes).
>
>  Before TOMAHAWK-858 is fixed, I would like to see a common approach
>  that we can take for all of the MyFaces projects so that it is easiest
>  for users to be familiar with.
>
>  Opinions, solutions?
>
>  Thanks,
>  Andrew
>



-- 

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces

Mime
View raw message