ofEventListeners: added size() method.

[roymacdonald]

This allows to things, checking not being out of bounds when removing a listener and checking if there are any registered listeners.
@arturoc @bakercp

[ofTheo]

So I'll confess I don't know enough about this to give meaningful input.
@arturoc has a response here: #6483 (comment)

Looking at the current class though it seems like it isn't designed to be used granularly, where you can remove / disable individual listeners. I am guessing from arturo's response that is the intent.

From what I can tell though ofEventListeners::unsubscribe( std::size_t pos ) doesn't really have much use without other helper functions like size() etc.

Would it be better not to have ofEventListeners::unsubscribe and just use multiple ofEventListeners for different groups of things you would want to enable/disable?

[arturoc]

oh sorry i read this as ofThreadChannel not ofEventListener. not sure we need this, the idea for ofEventListeners is just o have a quick way to remove several listeners at once at the end of the life of the container object. if you need more granularity than that then you can simple store them on a vector

[roymacdonald]

@arturoc 😄 Now your answer makes sense. As well I think that elliot was commenting something like this for the threaded channel as well so I understand your confusion, and sorry for confusing you.

as @ofTheo says, it is essentially because there is ofEventListeners::unsubscribe( std::size_t pos ) having a size() function becomes handy.
On the other side you can not put ofEventListener instances in a container, instead you need to use std::unique_ptr<of::priv::AbstractEventToken> which makes it quite cumbersome, and from its naming I would guess that it is not intended for being used elsewhere.

[ofTheo]

I think @roymacdonald you could probably use the system in ofEasyCam like this:

ofEventListeners mouseListeners;
ofEventListener updateListener; 

So you use ofEventListeners sort of as a block of things you would enable/disable all at once.
and ofEventListener for individual things like update events;

Also I think you can safely do:
vector < ofEventListener> listeners;

As the only data stored in ofEventListener is a unique_ptr so should be vector safe.

But I think maybe ofEventListeners::unsubscribe( std::size_t pos ) doesn't make sense without some other functions to support it.

[arturoc]

yeah i think unsubscribe doesn't make any sense either. even with a size mehtod once you remove one listener the indices for the rest change so even if you know how many there are in total it's really hard to know what you are unsuibscribing. i would mark this as deprecated for next release and don't add anything else to this class

[roymacdonald]

Hi, I use the ofEventListener and ofEventListeners in the way that @ofTheo suggests, and I agree that it makes no sense to have the unsubscribe method for the reason that @arturoc gives. Although, for what I do think it becomes useful is for checking if there are any listeners subscribed. For instance, I have a class with the following:

ofEventListeners mouseListeners;
void registerMouseEvents(){
    mouseListeners.push(.....);
....
}

void unregisterMouseEvents(){
    mouseListeners.unsubscribeAll();
}

But I'd like to have the following function as well:

bool isMouseRegistered(){
return ...?;
}

So, for such I would need to add a bool to my class that would track the calls to registerMouse() and unregisterMouse().
Even when this are just very few extra lines of code, it kinda makes more sense to me that the ofEventListeners somehow provides its status, for which the size() method makes sense. If you think that this makes no sense, it is ok, I just thought it could be handy and better for everyone.
Cheers

[ofTheo]

totally agree @roymacdonald and @arturoc :)
I think we should mark ofEventListeners::unsubscribe( std::size_t pos ) deprecated but also add in the size(); call in this PR.

I think being able to know if you have already called unsubscribeAll() or not makes sense and as you said Roy it would be cleaner to track/use this way.

Maybe you could add the deprecation note to this PR @roymacdonald ?

[roymacdonald]

@ofTheo sure. I'll add the deprecation note later today

[roymacdonald]

@ofTheo just pushed a commit with the deprecation message and updated everything else to the current master

[arturoc]

i don't think that size is a good idea if we are not allowing access to the internals at all. perhaps empty() is better, it' allows to understand if there's something registered or not but is not related the number of listeners registered which is not used anywhere else

[roymacdonald]

makes a lot of sense. I'll change it for empty

[ofTheo]

Thanks @roymacdonald !
Merging to master as its a new feature.