users@javaserverfaces-spec-public.java.net

[jsr344-experts mirror] [jsr344-experts] Re: FacesContext API review

From: Edward Burns <edward.burns_at_oracle.com>
Date: Fri, 15 Mar 2013 19:42:54 -0700

>>>>> On Wed, 13 Mar 2013 10:54:39 -0400, Andy Schwartz <andy.schwartz_at_oracle.com> said:

AS> Gang -
AS> More API review comments...

AS> Index: javax/faces/context/FacesContext.java
AS> ===================================================================
AS> --- javax/faces/context/FacesContext.java (revision 8845)
AS> +++ javax/faces/context/FacesContext.java (revision 11719)

AS> + /**
AS> + * <p class="changed_added_2_2">Return the list of resource library
AS> + * contracts that have been calculated
AS> + * to be appropriate for use with this view, or {_at_code null} if
AS> there are
AS> + * no such resource library contracts. The list returned by this
AS> method
AS> + * must be immutable. For backward compatibility with implementations
AS> + * of the specification prior to when this method was introduced, an
AS> + * implementation is provided that returns {_at_code null}.
AS> Implementations
AS> + * compliant with the version in which this method was introduced must
AS> + * implement this method as specified.</p>
AS> + *
AS> + * @throws IllegalStateException if this method is called after
AS> + * this instance has been released
AS> + *
AS> + * @since 2.2
AS> + */
AS> + public List<String> getResourceLibraryContracts() {

AS> When introducing new methods that return collections, we should:

AS> - Spec that the returned collection is immutable (if possible).
AS> - Spec that a non-null return value is guaranteed.
AS> - Either mark the new method as abstract, or, if we're not able/willing
AS> to do that, provide a default implementation that returns the empty
AS> collection.

AS> Code that wants to distinguish between empty and non-empty cases can
AS> easily do so. Code that does not care is not polluted by the null check.

Will do.

AS> + public void setResourceLibraryContracts(List<String> contracts) {

AS> I really don't like the approach of exposing a public setter with such
AS> restrictions, particularly because we need to keep multiple pieces of
AS> state coordinated.

I understand, but this one did have significant input from Frank and
Leonardo, so I'm loath to change it right now.

AS> + public ComponentModificationManager getComponentModificationManager() {

AS> As mentioned in my ComponentModificationManager email, I would strongly
AS> prefer that we kill off this API and revisit in 2.3.

Yes, done.

r11754

Ed