You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martijn Dashorst <ma...@gmail.com> on 2012/09/27 11:28:17 UTC
Make attribute modifier a temporary behavior?
I encounter the following code quite often in our company's codebase:
@Override
protected void onBeforeRender() {
add(AttributeModifier.replace("class", someCondition ? "someValue"
: "someOtherValue"));
}
This is of course a (often a minor) memory leak: each time the
component gets rendered, a new attribute modifier is added to the list
of behaviors, each overwriting the class attribute. The correct code
would of course be:
@Override
protected void onInitialize() {
add(AttributeModifier.replace("class", new
AbstractReadOnlyModel<String>() {
public String getObject() {
return someCondition ? "someValue" : "someOtherValue";
}
}));
}
Another solution would be to make the attribute modifiers temporary.
Problem with that is silent failures of attributemodifiers added in
onInitialize and constructors or guarded by hasBeenRendered().
My main question is: do you encounter this kind of attribute
modification in your application code? Were you aware of this problem?
Martijn
PS. I guess it is time to write some blog articles about this...
Re: Make attribute modifier a temporary behavior?
Posted by Carl-Eric Menzel <cm...@wicketbuch.de>.
On Thu, 27 Sep 2012 11:28:17 +0200
Martijn Dashorst <ma...@gmail.com> wrote:
> I encounter the following code quite often in our company's codebase:
>
> @Override
> protected void onBeforeRender() {
> add(AttributeModifier.replace("class", someCondition ? "someValue"
> : "someOtherValue"));
> }
>
> This is of course a (often a minor) memory leak: each time the
> component gets rendered, a new attribute modifier is added to the list
> of behaviors, each overwriting the class attribute. The correct code
> would of course be:
This is simply a bug, in my opinion, and there's an easy fix that you
have already shown.
Changing AttributeModifier to become temporary would be a silent
behavior change that will very likely bite many more users than those
who have code with the shown bug. I haven't seen this in our codebase
at all, for example. But I'd certainly scratching my head if these
modifiers suddenly were to disappear after the first request.
So, my opinion is: -1. Better to improve documentation for this.
Carl-Eric
Re: Make attribute modifier a temporary behavior?
Posted by Martin Grigorov <mg...@apache.org>.
On Thu, Sep 27, 2012 at 1:00 PM, Martijn Dashorst
<ma...@gmail.com> wrote:
> On Thu, Sep 27, 2012 at 11:46 AM, Martin Grigorov <mg...@apache.org> wrote:
>> See https://github.com/l0rdn1kk0n/wicket-bootstrap/tree/master/library/src/main/java/de/agilecoders/wicket/markup/html/bootstrap/behavior
>>
>> There are few CssClassName* classes which are a bit more sophisticated
>> than Wicket's AttributeModifier.
>
> That is nice, but doesn't address the memory leak issue.
By making the behavior temporary you will solve the memory issue but
then the #onInitialize() use case will become broken. The attribute
will be set only for the first render of the component. Any further
re-renders will not have it.
>
> Martijn
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com
Re: Make attribute modifier a temporary behavior?
Posted by Martijn Dashorst <ma...@gmail.com>.
On Thu, Sep 27, 2012 at 11:46 AM, Martin Grigorov <mg...@apache.org> wrote:
> See https://github.com/l0rdn1kk0n/wicket-bootstrap/tree/master/library/src/main/java/de/agilecoders/wicket/markup/html/bootstrap/behavior
>
> There are few CssClassName* classes which are a bit more sophisticated
> than Wicket's AttributeModifier.
That is nice, but doesn't address the memory leak issue.
Martijn
Re: Make attribute modifier a temporary behavior?
Posted by Martin Grigorov <mg...@apache.org>.
See https://github.com/l0rdn1kk0n/wicket-bootstrap/tree/master/library/src/main/java/de/agilecoders/wicket/markup/html/bootstrap/behavior
There are few CssClassName* classes which are a bit more sophisticated
than Wicket's AttributeModifier.
On Thu, Sep 27, 2012 at 12:28 PM, Martijn Dashorst
<ma...@gmail.com> wrote:
> I encounter the following code quite often in our company's codebase:
>
> @Override
> protected void onBeforeRender() {
> add(AttributeModifier.replace("class", someCondition ? "someValue"
> : "someOtherValue"));
> }
>
> This is of course a (often a minor) memory leak: each time the
> component gets rendered, a new attribute modifier is added to the list
> of behaviors, each overwriting the class attribute. The correct code
> would of course be:
>
> @Override
> protected void onInitialize() {
> add(AttributeModifier.replace("class", new
> AbstractReadOnlyModel<String>() {
> public String getObject() {
> return someCondition ? "someValue" : "someOtherValue";
> }
> }));
> }
>
> Another solution would be to make the attribute modifiers temporary.
> Problem with that is silent failures of attributemodifiers added in
> onInitialize and constructors or guarded by hasBeenRendered().
>
> My main question is: do you encounter this kind of attribute
> modification in your application code? Were you aware of this problem?
>
> Martijn
>
> PS. I guess it is time to write some blog articles about this...
--
Martin Grigorov
jWeekend
Training, Consulting, Development
http://jWeekend.com