Monday, September 17, 2007

Trinidad versus Tomahawk component architecture

I have been using Tomahawk quite a while and building several custom components with it. Relatively recently, I have incorporated Trinidad in a new project I am working on due to its AJAX support and large number of components.

Yesterday, I attempted to incorporate maven-faces-plugin into my project from the Trinidad code base. After trying it out, I have decided to back it out of my project, partially due to the amount of work, and mostly due to the architecture of Trinidad components.

Why is Tomahawk a better component development platform?

Yes, a bold statement, but I wanted to get your attention. Trinidad has a great offering, a lot of components, AJAX support and a good set of APIs, but there may be a large architectural flaw. This flaw is the architecture of Components and renderers around the FacesBean.

Comparison:

What I found is that Tomahawk follow an OO paradigm, but Trinidad breaks OOD standards. One of the major aspects of effective OOD is encapsulation. This basically means that the functionality an Object provides comes from the component itself. Tomahawk has a special map behind the getAttributes() method of each component. This map delegates the attributes to the property getter and setter methods of the component. Take for example:

// MyComponent comp; String value = comp.getMyProperty(); value = comp.getAttributes().get("myProperty");

These two lines of code are identical in terms of functionality, they both call "getMyProperty()" on MyComponent. This means that component developers know that the getter and setter methods of their component will be called, and they have the control to provide any functionality that is needed besides just storing and retrieving a value (they can manipulate values, check for illegal arguments, etc.). This illustrates a good comprehension of encapsulation, that the functions of MyComponent provide the functionality of MyComponent.

Trinidad works in a completely different way. Each UIXComponent has a FacesBean instance that is tied to a static Type that dictates what values are stored during state processing of the component. The getter and setter methods of the components retrieve and store values into this FacesBean. The break in encapsulation is that this FacesBean is made public. That means that people can access the FacesBean and get values without a reference to the component. Encapsulation is lost, the component no longer has any control over the functionality for its own properties. To make matters worse, all of the Trinidad JSP Tag classes and the Trinidad renders use this FacesBean directly, taking the component completely out of the picture.

Code generation

There were other issues that I had with the maven-faces-plugin. The biggest, is that this plugin drastically affects your project layout and architecture. Unlike the Tomahawk code generator which injects code into a component's source, Trinidad actually generates the entire component. This means that the Trinidad components have to be their own maven project. Furthermore, there is a two step process which requires a third project (i.e. trinidad-build, trinidad-api and trinidad-impl). After refactoring most of my code into this layout, the plugin still didn't work. It wasn't generating the taglib.xml files. Unfortunately the plugin is not documented

Conclusion

My conclusion from my research is to not use maven-faces-plugin for my project. For my custom components that extend Trinidad components, I am forced to use the FacesBean architecture or else risk breaking the Trinidad renderers, or at least not being compliant with the design.

To the Trinidad team:

Adam and the Trinidad team, what do you feel about inversing the FacesBean? What I mean by this, is that currently the get/set component methods store the values into the FacesBean. What about taking the design of the Tomahawk components and having the FacesBean call the get/set methods on the component to get their values and have the get/set methods of the component actually do the work? Since the code is generated, the cascade of the change shouldn't be too bad. Only people that have extended these components without using maven-faces-plugin would be affected.

Since this would break backwards-compatibility, Trinidad's major version could be bumped.


Updates and responses to comments

2007-09-17

Simon,

Yes, I know that the performance will be better and the generated code smaller, but is that a good reason for breaking standard OOD? With this model, the component's properties are reduced to the security and functionality of public fields.

I am a very strong believer in usage of OOD for Java objects and the current design forgoes the UIComponent design by having the renderers bypass the component and the component attributes by delegating component properties to the FacesBean.

Is the efficiency of FacesBean worth the cost of bypassing encapsulation and reducing a component's methods to not much more than a Map?

JDK 1.5 has made reflection much faster, and if the Method references were cached, it would be a much smaller performance hit. Take EJB3 for example, it uses reflection plus annotations to get and set values for database access. Even though there is a performance hit, the API remains viable for production use.

Instead of using FacesBean, renderers could use the attribute map instead, this would ensure that renderers remain component agnostic. For example:

public void encodeBegin(FacesContext facesContext, UIComponent comp) { Map attrs = comp.getAttributes(); Renderer delegate = getMyRendererDelegate(); delegate.renderStuff(facesContext, attrs); }

Or

public void encodeBegin(FacesContext facesContext, UIComponent comp) { Map attrs = comp.getAttributes(); String styleClass = toString(attrs.get("styleClass")); } protected String toString(Object obj) { return obj == null ? null : obj.toString(); }

In these crude examples, the renderer doesn't care what the component is, only that the component may (or may not) provide a specific attributes. In the first example, the delegation renderer can access the attributes to get a styleClass for example. The second example shows a class accessing a property via the attributes

With this paradigm, a renderer developer may choose to access the component directly (cast the component to one it supports) or use the attributes. If it uses the attributes, it choose to be able to support behaviors rather than types. In this way, renderers that are used as delegates could simply rely on the attribute map.

In terms of performance, the attribute may could cache, in a transient variable, the Method to call for the get and/or set property accessors. The component could choose to use FacesBean, or just store the value in a member variable. I know there is some performance overhead, but this is a side effect of a good encapsulation with the flexibility that you desire from the loose-binding a.k.a. late binding you desire for renderers.

2007-09-17

Adam,

The reason FacesBean violates OOD is that it delegates the behavior of the UIComponent to the FacesBean. The UIComponent no longer has any control over its own properties and is no longer aware of external accesses to its own data. Also, the way that an object chooses to serialize its data (saveState, restoreState behaviors in JSF), is an internal, not external function.

With the FacesBean being a public object, a piece of code can easily change the value of a property to something illegal, and the component has no opportunity to validate the new value.

Lastly, the component has no control over removing attributes. With proper use of encapsulation, a component could remove an attribute, maintaining the get/set properties as deprecated but store the value elsewhere. With the FacesBean, the component now is permanently bound to the data that it serializes, as people now code against the PropertyKey names instead of the exposed properties. The way that the FacesBean architecture is made, the private fields of UIXComponent have been made public.

2007-09-17

Simon,

Good point on the sub-classing FacesBean, perhaps I should give that option more thought. If I have the incentive and time, I may decide to create a new blog entry on customizing the FacesBean to create validation methods, the ability to create access methods (handle set/get) as a proof of concept.

5 comments:

SimonLessard said...

Hello Andrew,

I agree FacesBean represents a break on encapsulation to some level since you an alter the attribute value without calling the accessors. However, there's at least 2 major advantages with FacesBean architecture:

1. Reduction of boilerplate code since FacesBean automatically deals with value expressions, state saving, etc.

2. Far more efficient than getAttributes() map is you make loosely coupled renderers since PropertyKey is more or less a dynamic enum granting an access speed comparable to arrays. However, if the renderer is strongly coupled to the component class, you'll get a small performance cost with FacesBean. However, Trinidad often delegate rendering of some component parts to other renderers and strongly coupling the renderers to the component class would prevent that.

Note: The contract of getAttributes() is still enforced, calling a get on the map will call the getter in the component that will in turn call the FacesBean to get the current value. This is, on the other hand, very inefficient, but Trinidad does not use attribute map in many places.

About maven-faces-plugin, you don't have to use FacesBean architecture with it. MyFaces 1.2.x use it to generate its components.


Regards,

~ Simon

Adam Winer said...

Simon's already listed most of the advantages of the FacesBean approach, which is why we went that way. Performance was foremost on our minds. The ability to decouple rendering from a specific component was second highest priority. Elimination of boilerplate was third highest.

Also, to emphasize peformance more: the approach of making the bean accessors "do the work" has painful implications for state saving performance. Every attribute is stored, whether or not it's been set. FacesBean is much more efficient with sparsely-set components (the most common case by far).

As a historical footnote, the FacesBean approach also made it possible to bootstrap the old UIX codebase (see the UINode code, etc.) into JSF land without a true violation of encapsulation.

I don't understand why you see FacesBean as a massive violation of OO principles - in particular, for FacesBean-based components, the FacesBean is the contract, and the accessors are mere convenience functions. As long as you think the accessors are the contract, yes, you'd think it's violating encapsulation.

BTW, the term encapsulation is entirely dependent on your scope. If you insist that the scope is the UIComponent instance, then it's violating encapsulation in this artifical sense. But that's your arbitrary choice. The encapsulation is perfectly fine on the combination of the FacesBean and the UIComponent, which are a united pair, and there's nothing wrong in OO with such a strategy!

So, yes, it's different. But I'm completely happy with the choices we've made, and think Trinidad has a very coherent, considered architecture.

Adam Winer said...

"The reason FacesBean violates OOD is that it delegates the behavior of the UIComponent to the FacesBean".

No. Delegation is perfectly acceptable OOD. At most, it's breaking JavaBean concepts, but JavaBeans != OO.

"With this model, the component's properties are reduced to the security and functionality of public fields."

That's false. FacesBean is in no practical way worse than the getter/setter design seen in 99.9% of JSF components - if we're going to talk OO hardcore, the entire JavaBean approach is a lousy OO approach. As an aside, FacesBeans today don't do much validation of their input - it'd be easy and worthwhile to add more validation - though for example, they make it trivial to note that an attribute doesn't support EL, and block attempts to do so, which I suspect most component developers forget.

It's also entirely OK for a component to store state that is entirely private off the FacesBean. But if you're exposing a public getter + setter, give up the notion that you've got some super private implementation detail.

"Is the efficiency of FacesBean worth the cost of bypassing encapsulation and reducing a component's methods to not much more than a Map? JDK 1.5 has made reflection much faster, and if the Method references were cached, it would be a much smaller performance hit. "

YES. The efficiency is TOTALLY worth it. I don't know how much time you've spent profiling JSF applications and other similar frameworks, but I've spent tons. The time spent looking in component attribute maps is HUGE. Reflection is only a fraction of that - it is very easy for HashMap.get() to become a dominant part of overhead, and the optimizations in FacesBean to make PropertyKeys faster to retrieve (especially around the indices) are major wins.

"With the FacesBean, the component now is permanently bound to the data that it serializes, as people now code against the PropertyKey names instead of the exposed properties."

As opposed to getAttributes().get()???

"Also, the way that an object chooses to serialize its data (saveState, restoreState behaviors in JSF), is an internal, not external function."

That's ridiculous. Everyone hates writing that boilerplate code for this internal function, and it's inefficient splitting this implementation among the class hierarchy, layering arrays in arrays in arrays. The FacesBean approach to state saving is far better.

SimonLessard said...

Andrew,

Data alteration control is still possible with FacesBean by using an internal FacesBean decorator to override getProperty and setProperty methods. You then decorate the FacesBean returned by getFacesBean with it and you'll be able to do whatever you want with incoming requests.

That pattern makes it even easier to add, for example, a logging behavior as you can enable it for all properties in a single method.

As for validations, you can enforce whatever you want in the decorator using a Map<PropertyKey, Method> to not hit the performance. I agree it's not as intuitive as getter/setter, but it works.

Pete Muir said...

You might be interested in the CDK that comes with RichFaces - keeps the standard UIComponent. It does require its own project though.

A bit short on docs atm, but some are being worked on.