Hi,
I had a window of time to try to do the "has" vs "is" refactor for
ItemRenderers.
However, I didn't get very far because as I started looking through the
ItemRenderer code, I realized that there are other things that could use a
refactor as well. IMO, there is too much just-in-case code in current
renderers. IOW, too many if statements. For example: getLabelFromData is full
of just-in-case code
Also:
-Not all renderers should need an "index" property
-There are both dataField vs labelField properties and I'm not sure anyone is
using dataField
-The renderer factories are setting labelFIeld on each renderer.
Also, there are also a ton of DataItemRendererFactory variants, many containing
duplicate code.
And while I'm thinking about it, we should document why "data" set after
parenting, which is different from our recommended lifecycle (normally we set
properties then parent). The reason is that the setting of the data property
triggers bindings once, which can be evaluated against a known CSS tree, which
is more efficient.
Then there is the itemRendererParent property which is poorly named and I don't
think it is being used properly.
So, I'm leaning towards a more impactful refactor. "Has" is not fast right now
and I don't want to build a faster "has" capability until we see whether it
works at all, but we also don't want to skew the data by asking too many
unnecessary "has" questions.
Some ideas for the refactor:
A) The renderers should pull additional information (labelField,
labelFunction), although there is a concern that could be expensive to find the
additional info. On the other hand, fewer pieces need to touch the additional
information so maybe it is more PAYG and better encapsulated
So, If each renderer got a 'data' and the itemRendererParent) it would get
additional information via:
myLabelFunction = itemRendererParent.strand.labelFunction
or maybe
myLabelFunction = itemRendererParent.strand.model.labelFunction
B) One of the reasons there are so many DataItemRendererFactory classes is
because creating renderers is a loop and needs to be fast so the variations are
effectively inlined. However, I've noticed that Harbs has been refactoring
code into shared functions (like sendEvent). Do we have data that function
call overhead is not as big a factor as it was in Flash? Or that we get gains
back from the browser optimizing (maybe via JIT) hot code paths?
If we're better off sharing more code, then all of the DataItemRendererFactory
variants could extend some base class and have overridable methods or a few
more composited beads to abstract the differences in the variants. I think the
4 steps in the Factory are:
1) get next item
2) create the renderer
3) parent it
4) set data on the renderer
Currently only #2 is done with a bead. #1 and #4 probably could be as well (or
by overriding a method in the base class).
The challenge is for #4 to try to allow different sets of properties to be
passed to the renderer. The index should be passed if the renderer wants it,
and the data and the itemrendererview/parent.
C) The renderers may also need a refactor. StringItemRenderer should presume
that data is a String. It should not use getLabelFromData. DataItemRenderer
should assume there is a data object and a dataField/labelField. That's why
there is a TextItemRendererFactory and a DataItemRendererFactory. The former
assumes the data is a String, the latter assumes the data is an instance of an
object.
A renderer in the Express package can have more if statements and check if
labelField is being used or not.
D) We might look at abstracting the computation of the label for a renderer.
That feels like it would be too heavy in many cases, but right now we call
getLabelFromData anyway.
Thoughts?
-Alex
