Attach event listeners upon attribute changes

[ZombieRaccoon]

I implemented the feature we discussed in #188, along with a couple of tiny improvements that presented themselves along the way.

A couple of notes about the end result:

• I went with a simple linear search in EventDispatcher::DetachEvent, since I believe that only a relatively high listener count would warrant using binary search and I don't think that a UI library can reach that high a listener count for it to make the extra effort worthwhile.
• In Element::OnAttributeChange it's necessary to traverse the whole dictionary to find attributes that start with "on", so for effectiveness' sake I converted the entire thing to build off of that.

[mikke89]

Thank you! Should be a nice little improvement.

I'm not sure it's working entirely correct in terms of replacing old on... attributes, see my comments for details. And I think we should also remove the listener when users remove the on... attribute.

We should add some tests to make sure it works as intended to the UnitTests suite, so that we know replacement and removal works properly. I'm also interested to see if the onload attribute is fired properly.

I did benchmark this just to be sure and I couldn't measure any regression, so that is all good!

[ZombieRaccoon]

Trying to implement removal and taking the ability to set listener origin off the public interface actually gave me an idea for a different approach, which I think you'll also like. I'll push the commit soon.

[mikke89]

Yeah, this approach should work fine as far as I can tell.

Could you perhaps add some unit tests as well? Thanks!

[ZombieRaccoon]

Yeah, this approach should work fine as far as I can tell.

Could you perhaps add some unit tests as well? Thanks!

Absolutely! Your original request didn't go unheard, it's just that I didn't manage to fit both into my time. I'll make sure to follow up with unit tests.

[ZombieRaccoon]

@mikke89 have you thought about using a mocking framework? I'm in the midst of writing the unit tests and it'd come in quite handy. Seeing as the project is using doctest as its unit testing framework, I'd suggest trompeloeil based on some research on the matter. If you were to be in agreement, I'd integrate the library under Tests\Dependencies.

[mikke89]

I don't really have any experience with mocking frameworks. I'm not really convinced by the reading I did on it, but if you think we can express some tests better I'll trust you on that. The library you mention looks good in terms of integration and licensing.

[ZombieRaccoon]

In my opinion, the most important thing that mocking frameworks save you is writing custom fake implementations like the one I resorted to in FakeEventListenerInstancer.h. The main goal of such constructs is to verify certain dynamic characteristics of software - whether the InstanceEventListener method of the registered EventListenerInstancer implementation is invoked with certain parameters, in this particular case - that would be extremely tedious work otherwise because of the inherent opaqueness of object-oriented architectures from certain points of entry.

I'll implement the same thing using the library I've mentioned and you'll immediately see the difference in readability and likely recognize possible future use or even potential test refactors stemming from the opportunities its expressive power provides.

[mikke89]

I see, please go ahead with it, I'm eager to see the results. Good work on the tests so far, thanks for the thorough work!

[mikke89]

There you go, compiler finally gives in ;)

Let me know when you feel happy with it by marking it as ready. Could you clean up the commit history? I'm thinking one commit for the library integration, one or two for the core changes, and one or two for the unit tests. Thanks!

[ZombieRaccoon]

Yeah, that warning from GCC was interesting to say the least. Anyway, I believe that the deed is done. I do agree that the commit history got a little cluttered, so I cleaned it up.

[mikke89]

Looks good, thanks again!