On Thursday, 7 November 2013 at 13:45:57 UTC, Dicebot wrote:
Time to move forward with some more reviews :)
Hi,
Apologies, I missed this review thread. So, some questions /
observations:
IMO, the effort to disallow third parties to "emit" signals on
your object by far exceeds its worth.
For this purpose, you have needed:
- string mixins
- ...which declare, by convention, a field with a name not
explicitly given by the user
- ...with a workaround for when they don't work
- an enumeration type
- two different signal struct types
- a boatload of documentation
- a nontrivial cognitive effort on the user to grok all of the
above
This little feature steals too much focus, and IMO is not worth
it.
As a compromise, have you considered template mixins?
mixin template M()
{
public void pub() {}
private void pri() {}
}
struct S
{
mixin M m;
}
Trying to access S.m.pri() from another module fails as expected.
The downside is that the ".m" is not required, so the mixin
members will "spill over" the parent object's field namespace.
This implementation supersedes the former std.signals
I don't think comparisons of older versions of a module belong in
the module's documentation.
a.valueChanged.connect!"watch"(o); // o.watch is the slot
This is really ugly, but I don't see any way to improve it
either. Maybe if the type of &classInstance.method was actually a
special "pointer-to-class-method" type that implicitly
down-casted to a delegate, it would be possible to safely use
&o.watch instead...
dg must not be null. (Its context may.)
I don't think the context will ever be null (unless you take the
address of a null class/struct pointer's method). (toDelegate
uses the context pointer to store the function pointer, and the
function pointer to store a template instance according to the
function's arguments.)
It implements the emit function for all other functionality it
has this aliased to RestrictedSignal.
Missing some punctuation, I think.
The idea is to instantiate a Signal privately and provide a
public accessor method for accessing the contained
RestrictedSignal.
I had a hard time understanding this line the first time. Please
reword without using "The idea is", because it's not clear what
"the idea" is for - usage? Rationalization of implementation
method?
Use this overload if you either really, really want strong ref
semantics for some reason
Was the emphasis really necessary? It makes complete sense to
build an asynchronous-event-model application using 100% only
"strong connections" (delegates or delegate arrays). I don't
remember ever NEEDING weak-referenced callbacks.
