gnustep / libs-base

The GNUstep Base Library is a library of general-purpose, non-graphical Objective C objects.
https://www.gnustep.org/
GNU General Public License v2.0
935 stars 282 forks source link

NotificationCenter removeObserver #396

Closed williameveretteggplant closed 6 months ago

williameveretteggplant commented 6 months ago

This modifies the NotificationCenter class to do the following:

This replaces the previous implementation which checked the class of the observer as it was being removed, which would cause a crash if the observer had been deallocated.

qmfrederik commented 6 months ago

The MinGW GCC build errors are weird. I assume it's because you are using an iterator in your code. What happens if you change:

  for (id listObserver in _retainedObserverArray) 
    {
      if (listObserver == observer)
        {
      needToRelease = YES;
          break;
    }
    }

to

for (int i = 0; i < [cs count]; i++)
  {
     if ([_retainedObserverArray objectAtIndex: i] == observer)
       {
      needToRelease = YES;
          break;
       }
  }

or just

if ([_retainedObserverArray containsObject:observer)
  {
    needToRelease = YES;
    break;
  }

?

rfm commented 6 months ago

The best code for NSArray would use the -indexOfObjectIdenticalTo: since (as well as being a portable alternative) this is simpler and more efficient than the use if fast enumeration.

I'm not completely sure that holding the observer in a mutable array is the best approach though; an array is efficient if there are a small number of observers, but doesn't scale well if there are a lot of observers being added and removed. To scale, the function API of NSHashTable API (or using the GSI macros) would probably be fastest.

williameveretteggplant commented 6 months ago

I have taken suggestions into account and updated.