TextField value ghosting bug

Vaadin 6 / 7 / 8 Framework UI
1

This might be ‘user’ error but it took me some time to figure this out. This is one of the ‘rare’ times I miss Java Swing but I’m thankfully I have a ‘tiny’ glimpse of what is behind the Vaadin curtain.

Here is the code that was failing and look at Line item #3 below:

[code]
_updateStates.addClickListener(new ClickListener() {

        StringBuilder selectedStates = new StringBuilder();

        @Override
        public void buttonClick(ClickEvent event) {

            @SuppressWarnings("unchecked")
            Set<String> selections = (Set<String>) _states.getValue();
            Object states = selections.toArray();

            LOG.info("TwinCol getValue list=" + selections);
            
            final int size = selections.size();
            final int nextToLast = size - 1;

            for (int i = 0; i < size; i++) {
                selectedStates.append(states[i]

);
if (i < nextToLast) {
selectedStates.append(", ");
}
}

            LOG.info("selected list=" + selectedStates.toString());

            _parentView._states.setImmediate(true);
            _parentView._states.setReadOnly(false);
            _parentView._states.setValue(selectedStates.toString());
            _parentView._states.setReadOnly(true);
        }
    });

[/i]
[/code]I have a TwinColumn list of states where the State name is shown and after selection I put the State Postal code in a read-only TextField.

So during testing when I logged this I got the following:

2015-06-30 10:41:56 INFO StatesWindow.buttonClick:[109]

  • TwinCol getValue list=[AZ, AR]

2015-06-30 10:41:56 INFO StatesWindow.buttonClick:[121]

  • selected list=AZ, AR
    2015-06-30 10:42:00 INFO StatesWindow.buttonClick:[109]
  • TwinCol getValue list=
    2015-06-30 10:42:00 INFO StatesWindow.buttonClick:[121]
  • selected list=AZ, AR

Notice how the StringBuilder even though it is ‘newed’ is GHOSTING content!?!?! How does it STILL have data? The only thing I can think of is ClickListener and ClickEvent BOTH have a copy and since the ‘inner’ version is null my ‘outer’ version falls through? Not sure and I don’t have time to debug it but I did want to share this diamon in the rough. :slight_smile:

The fix is simply MOVING where StringBuilder is constructed! See below and notice the last log item below is NULL as compared to the last log item shown above.

[code]
_updateStates.addClickListener(new ClickListener() {

        @Override
        public void buttonClick(ClickEvent event) {

            @SuppressWarnings("unchecked")
            Set<String> selections = (Set<String>) _states.getValue();
            Object[] states = selections.toArray();

            LOG.info("TwinCol getValue list=" + selections);

            StringBuilder selectedStates = new StringBuilder();
            final int size = selections.size();
            final int nextToLast = size - 1;

            for (int i = 0; i < size; i++) {
                selectedStates.append(states[i]

);
if (i < nextToLast) {
selectedStates.append(", ");
}
}

            LOG.info("selected list=" + selectedStates.toString());

            _parentView._states.setImmediate(true);
            _parentView._states.setReadOnly(false);
            _parentView._states.setValue(selectedStates.toString());
            _parentView._states.setReadOnly(true);
        }
    });

[/code]That little change now gives me the following:

2015-06-30 10:46:57 INFO StatesWindow.buttonClick:[107]

  • TwinCol getValue list=[AZ, AR]

2015-06-30 10:46:57 INFO StatesWindow.buttonClick:[120]

  • selected list=AZ, AR
    2015-06-30 10:47:04 INFO StatesWindow.buttonClick:[107]
  • TwinCol getValue list=
    2015-06-30 10:47:04 INFO StatesWindow.buttonClick:[120]
  • selected list=

The test case was selecting two states, adding to the list and then selecting NULL to clear them out.
Notice by moving the StringBuilder local data variable it changes EVERYTHING. :wink:

Bug? Feature?
You tell me, but I know which one I am sticking with so I can meet a code deadline. :wink:

Cheers

2

Hi!

It seems to me that on the first version you never clear the StringBuilder between events, and all the events thus just keep adding content. The ClickListener itself is the same instance through the whole thing, so any fields that belong to it are not initialized again after you first set it. On the second version you create a new StringBuilder for every event, so you get only the current state instead of the whole history of clicks.

3

Anna,

Thank you so much for the post. We will have to agree to disagree on this one. In the Java Swing world this is perfectly legal:

_updateStates.addClickListener(

new ClickListener()

 {
StringBuilder

selectedStates = new StringBuilder();


@Override
public void buttonClick(ClickEvent event) {

If I
new
 a ‘selectedStates’ variable within a “new ClickListener()” event than I would fully expect a “clean” copy or “empty” copy of the StringBuilder object. I would not expect it to
cache across session events
 as it was doing.

I’ve seen it do odd things like this in the past and it is ‘trivial’ to fix but it makes me wonder how this could ever happen.

Cheers,

Ed

4

Hi!

In Vaadin it works this way because it’s a perfectly normal anonymous inner class when used this way, absolutely no magic to it. ClickListener is a very simple interface. You can even make your UI (or any view/layout/whatnot) implement that interface and give the UI as a listener to every single Button within your UI, and it would make very little sense if this would lead to every field of the UI getting reinitiated at every click. That’s why you can make your own special listeners with very little fuss, and also it gives you a place to e.g. store the overall click count if you happen to need it, or maybe give different behaviour if the click happens after some other thing has happened first. You can create a dedicated ClickLogger class that implements ClickListener, and have all the logic in there instead of spilling it over every view where you want to use it. It’s a different approach to Swing world, yes, it but has its benefits :slight_smile:

5

Thanks Anna!