Signals and Slots

Frits van Bommel fvbommel at REMwOVExCAPSs.nl
Fri Nov 3 02:33:38 PST 2006


Some more suggestions, interspersed in the source:

 > // Special function for internal use only.
 > // Use of this is where the slot had better be a delegate
 > // to an object or an interface that is part of an object.
 > extern (C) Object _d_toObject(void* p);

If it's for internal use only, why isn't it private?

[...]
 > template Signal(T1...)
 > {
 >     /***
 >      * A slot is implemented as a delegate.
 >      * The slot_t is the type of the delegate.
 >      * The delegate must be to an instance of a class or an interface
 >      * to a class instance.
 >      * Delegates to struct instances or nested functions must not be
 >      * used as slots.
 >      */
 >     alias void delegate(T1) slot_t;
 >
 >     /***
 >      * Call each of the connected slots, passing the argument(s) i to 
them.
 >      */
 >     void emit( T1 i )
 >     {
 >         foreach (slot; slots)

No need to iterate over the *entire* array, this should do:
          foreach (slot; slots[0 .. slots_idx])

 >         {   if (slot)

If my other modifications are implemented, this check will be unnecessary

 >                 slot(i);
 >         }
 >     }
 >
 >     /***
 >      * Add a slot to the list of slots to be called when emit() is 
called.
 >      */
 >     void connect(slot_t slot)
 >     {
 >         /* Do this:
 >          *    slots ~= slot;
 >          * but use malloc() and friends instead
 >          */
 >         auto len = slots.length;
 >         if (slots_idx == len)
 >         {
 >             if (slots.length == 0)
 >             {
 >                 len = 4;
 >                 auto p = std.signals.calloc(slot_t.sizeof, len);
 >                 if (!p)
 >                     std.signals._d_OutOfMemory();
 >                 slots = (cast(slot_t*)p)[0 .. len];
 >             }
 >             else
 >             {


 >                 // Look for unused entry in slots[]
 >                 foreach (inout s; slots)
 >                 {
 >                     if (!s)                // found unused entry
 >                     {        s = slot;
 >                         goto L1;
 >                     }
 >                 }

Unnecessary O(n) operation that can be avoided by a little extra work in
disconnect() and unhook(), see below.

 >
 >                 len = len * 2 + 4;
 >                 auto p = std.signals.realloc(slots.ptr, slot_t.sizeof 
* len);
 >                 if (!p)
 >                     std.signals._d_OutOfMemory();
 >                 slots = (cast(slot_t*)p)[0 .. len];
 >                 slots[slots_idx + 1 .. length] = null;
 >             }
 >         }
 >         slots[slots_idx++] = slot;
 >
 >      L1:
 >         Object o = _d_toObject(slot.ptr);
 >         o.notifyRegister(&unhook);
 >     }
 >
 >     /***
 >      * Remove a slot from the list of slots to be called when emit() 
is called.
 >      */
 >     void disconnect( slot_t slot)
 >     {
 >         debug (signal) writefln("Signal.disconnect(slot)");
 >         foreach (inout dg; slots)

         foreach (inout dg; slots[0 .. slots_idx])

 >         {
 >             if (dg == slot)

 >             {   dg = null;

If we replace this line by:
               {  slots_idx--;
                  dg = slots[slots_index];
                  slots[slots_idx] = null;
then we don't need to check for empty slots in emit():
If slots.length == slots_idx, we can be sure the array is full.

(Unless there's a requirement for the slots to be
called in any particular order?)

 >
 >                 Object o = _d_toObject(slot.ptr);
 >                 o.notifyUnRegister(&unhook);
 >             }
 >         }
 >     }
 >
 >     /* **
 >      * Special function called when o is destroyed.
 >      * It causes any slots dependent on o to be removed from the list
 >      * of slots to be called by emit().
 >      */
 >     void unhook(Object o)
 >     {
 >         debug (signal) writefln("Signal.unhook(o = %s)", cast(void*)o);
 >         foreach (inout slot; slots)

         foreach (inout slot; slots[0 .. slots_idx])

 >         {

 >             if (slot.ptr is o)
 >                 slot = null;

Besides shrinking slots[0 .. slots_idx] to match, I have another remark 
here.
If slot is an interface delegate, this doesn't work. The test should 
probably be changed to:
             if (_d_toObject(slot.ptr) is o)
to handle this case.

 >         }
 >     }
 >
 >     /* **
 >      * There can be multiple destructors inserted by mixins.
 >      */
 >     ~this()
 >     {
 >         /* **
 >          * When this object is destroyed, need to let every slot
 >          * know that this object is destroyed so they are not left
 >          * with dangling references to it.
 >          */
 >         if (slots)
 >         {
 >             foreach (slot; slots)

             foreach (slot; slots[0 .. slots_idx])

 >             {
 >                 if (slot)
 >                 {   Object o = _d_toObject(slot.ptr);
 >                     o.notifyUnRegister(&unhook);
 >                 }
 >             }
 >             std.signals.free(slots.ptr);
 >             slots = null;
 >         }
 >     }
 >
 >   private:
 >     slot_t[] slots;                // the slots to call from emit()
 >     size_t slots_idx;                // used length of slots[]
 > }



More information about the Digitalmars-d mailing list