Possible issue with multiple short listeners

[guersam]

There are many onSomethingHappend with more than 1 callback like below (from here):

@inline def onItemSelected(f:  => Unit): V = {
  basis.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener {
    def onItemSelected(p1: AdapterView[_], p2: View, p3: Int, p4: Long): Unit = { f }
    def onNothingSelected(p1: AdapterView[_]): Unit = {  }
  })
  basis
}

As it invokes basis.setSomeListener which replaces previous listener, calling onNothingSelected with onItemSelected will cancel each other.

We might solve this with internal addSomeListener implementation. Do you have any idea to improve this?

[pocorall]

I wrote the listener-helper as you pointed out because:

• It is simple that not supporting multiple listener-helper :smile:
• It saves less line-of-code when using multiple listeners

For example, instead of this:

  basis.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener {
    def onItemSelected(p1: AdapterView[_], p2: View, p3: Int, p4: Long): Unit = { doSomething() }
    def onNothingSelected(p1: AdapterView[_]): Unit = {  }
  })

This saves LOC dramatically:

  basis.onItemSelected(doSomething())

Meanwhile, this can be an improvement as well, but less impressive:

  basis.onItemSelected(doSomething())
  basis.onNothingSelected(doAnother())

As you mentioned, addSomeListener approach can be a solution enabling this. But it costs runtime overhead on memory, CPU, and the size of compiled jar. I did not decided that it is worth doing in spite of these overheads.

I admit that it is not documented (non-) feature. One of the most pressing issue is documentation.

[guersam]

I agree. Then how about a macro shows a warning about using 'unsafe' listener in compile time?

[pocorall]

Developers might get too much warning. Warning usually indicates bad practice, but this is not the case. I think that proper documentation is better.

[pocorall]

Assigning the same listener multiple times is obviously a bad practice though. Is the Scala macro can handle this?

[guersam]

It will be hard unless the scope is narrow enough. How about to show compile-time warning by default, and allow disabling it by importing specific value? Might need some time though....

[pocorall]

If it takes some time, leave it and moving to other urgent issues would be wise. I will stay opened this issue.