Comments on the comprehensibility of the README, naming of classes, methods

[Ondina]

This is a great extension, and I think it will be useful. Everything looks fine and clean. The readme is perfectly clear to me.

But, just to continue our talk about disambiguation (sic), I would suggest we think some more about the naming of classes, methods and arguments: PayloadEvent , withValueClasses, valueClasses, valueObjects.

As I said, to me everything is clear, but I’m trying to look at it with the eyes of someone new to rl, who also doesn’t know anything about signals, and so on. And that’s just because you asked;)

I’ll think about alternative names, but I’m not sure I’ll find anything better than yours. Also, I’ll try to think of a weird scenario, where injecting foo and bar could result in a conflict with another mapping (maybe a named one, injector.map(String, 'foo').toValue(foo);)

I’ll report back.

[creynders]

@Ondina yeah I kind of agree. But I just wonder whether someone new to RL needs to understand this from the get-go. My experience with RL was: getting familiar to the core/framework and then running into walls, finding out people had written extensions for them and then getting to know those extensions.

Also, I’ll try to think of a weird scenario, where injecting foo and bar could result in a conflict with another mapping (maybe a named one, injector.map(String, 'foo').toValue(foo);)

It's something we can't do anything about I think. The same problem exists with the SignalCommandMap, if I'm not mistaken.

[Ondina]

Yeah, you’re right about the learning path to follow, but you can’t compare yourself to the majority of rl-users.
In practice, what happens is that many users expect to understand „everything“ from the get go, they don’t even try out an example or another, don’t read the documentation and so on. If something is unclear, they immediately come to the rl-forum and ask questions like these (roleplaying the noob) :

-Isn’t any custom event capable of carrying a payload? What’s the difference between a custom event and a PayloadEvent?

Possible answer: PayloadEventsExtension offers type safety. Your event, that extends PayloadEvent, accepts any number of parameters (..valueObjects) in the constructor. The only thing you have to do is to specify the type of the params within withValueClasses() method.

PayloadEventCommandTrigger takes care of mapping the valueClasses specified in withValueClasses() to your valueObjects.

-What are the advantages of injecting the valueClasses into a command over accessing the payload of a ‘normal’ custom event?

-What’s the advantage of using PayloadEvent over Using VOs (value object classes) as a payload for a custom event, which would guarantee that data is strongly typed?

-With a custom event, a payload can be made optional in the constructor, thus I can dispatch the event with or without passing the payload. I can’t dispatch a PayloadEvent without a payload.

As for the names, I couldn’t come up with something really good, but valueObjects makes me think of VOs classes. What about: …params or …payload instead of …valueObjects paramsTypes or payloadTypes instead of valueClasses withParamsTypes or withPayloadTypes instead of withValueClasses

But, on the other hand, since the terminology has been borrowed from as3-signals, maybe it should remain the same in this extension, so people accustomed to signals can see the resemblance?

I tried the extension in an example. The problem is, you cannot have 2 or more params of the same type:

var someString:String = "someValue";
var anotherString:String = "anotherValue";
dispatcher.dispatchEvent( new ConfiguredPayloadEvent( ConfiguredPayloadEvent.TYPE, someString, anotherString));
In ConfiguredPayloadEvent:
withValueClasses(String, String);

which issued an error: Warning: Injector already has a mapping for String|…… Error: Injector mapping override for type [class String] with name

I guess unmapping it prior to the new mapping is not an option or it’s not that easy to do.

[Ondina]

Addition to a possible explanation: withValueClasses() is optional.

withValueClasses() makes sure that each valueObject has the correct/expected type and that the number of valueObjects == the number of valueClasses (PayloadEvent.validateConfiguration() ).

Thus the following will throw an error: withValueClasses(String, Number, Array); dispatcher.dispatchEvent( new ConfiguredPayloadEvent( ConfiguredPayloadEvent.TYPE, 42, [1], "juju"));

This is correct: withValueClasses(String, Number, Array); dispatcher.dispatchEvent( new ConfiguredPayloadEvent( ConfiguredPayloadEvent.TYPE, "juju", 42, [1]));

Without withValueClasses() there is no restriction regarding the number and/or order of the arguments. PayloadEventCommandTrigger will map them to the correct type:

const ctor:Class = valueObjects[i]['constructor'] as Class; _injector.map(ctor).toValue(valueObjects[i]);

Of course, you can and should formulate it better than I did.

Note: I think _valueClasses && validateConfiguration(); is obsolete on line 41 in PayloadEvent.

[creynders]

Hey @Ondina thanks a lot for the extensive explanation!

-What are the advantages of injecting the valueClasses into a command over accessing the payload of a ‘normal’ custom event?

The main reason I wrote the extension is that with normal events, you tie the commands to the event types since you need to inject them to access the value objects. I really hate that, since it limits their re-use. Commands should obviously be tied to the VO type, but not to the event carrying the VO. For instance if you have two events that carry the same payload type and need to trigger the same command you're in trouble. Obviously there are other solutions for those cases, but hey always involve rewriting parts, with payload events that won't be necessary. Also if you start with events and a certain moment decide you'd rather use signals you'll be rewriting lots of stuff. Command should be blissfully ignorant of their triggers, whether they're signals, events or direct invocations.

As for the names, I couldn’t come up with something really good, but valueObjects makes me think of VOs classes.

That's kind of the idea. They should really be used with VO's. That's why...

I tried the extension in an example. The problem is, you cannot have 2 or more params of the same type

... this isn't a problem. It's meant for carrying VO's. It's exactly the same with signals BTW. But it definitely means I should rewrite the README and not use primitive types as an example, since otherwise people will think that's what it's for.

since the terminology has been borrowed from as3-signals, maybe it should remain the same in this extension, so people accustomed to signals can see the resemblance

Exactly.

I think _valueClasses && validateConfiguration(); is obsolete on line 41 in PayloadEvent.

No, it's to allow for a more flexible super constructor calling. This way you can write your custom PayloadEvent constructor like this:

withValueClasses(Foo, Bar)
super(type, valueObjects);

but also like this

super(type, valueObjects);
withValueClasses(Foo, Bar)

[Ondina]

Some more thoughts. I was trying to think of different scenarios that would cause users to ask why the extension is not working as expected, so you can write your README to address such situatiations.

First scenario:

Mappings injector.map(SomeVO).asSingleton();

SomeVO private var _someString:String; private var _someNumber:Number; //getters //setters

SomeMediator var someVO:SomeVO = new SomeVO(); someVO.someString = “someValue“;

dispatcher.dispatchEvent( new ConfiguredPayloadEvent( ConfiguredPayloadEvent.SOME_TYPE, someVO));

AnotherMediator [Inject] public var someVO:SomeVO;

SomeCommand [Inject] public var someVO:SomeVO;

If you map SomeVO asSingleton, so it can be injected into AnotherMediator, then the injector complains about mappings override when PayloadEventCommandTrigger is trying to map the class.

If you don’t map SomeVO, then, of course, it can’t be injected into AnotherMediator.

You could do this to prevent the error: if(_injector.hasMapping(payloadEvent.valueClasses[i])) _injector.map(payloadEvent.valueClasses[i]).toValue(valueObjects[i]);

but, when the command is done, the injector removes the mappings, thus the injector can’t inject SomeVO into AnotherMediator. Of course, you can’t know which classes should be unmapped and which ones shouldn’t.

SomeVO could be any other class that you wanted to be injected into different actors in your app.

Another scenario:

SomeCommand [Inject] public var someVO:SomeVO;

[Inject] public var anotherVO:AnotherVO; .. SomeMediator

In someMethod(): dispatcher.dispatchEvent( new UnconfiguredPayloadEvent (UnconfiguredPayloadEvent.SOME_TYPE, someVO, anotherVO)); //someVO and anotherVO get unmapped after the command has run

In anotherMethod(): dispatcher.dispatchEvent( new UnconfiguredPayloadEvent (UnconfiguredPayloadEvent.SOME_TYPE, someVO)); //Injector will miss a mapping for anotherVO.

The main reason I wrote the extension is that with normal events, you tie the commands to the event types since you need to inject them to access the value objects.

Right, you can’t inject both events into SomeCommand to access their payloads, when you have something like this:

commandMap.map(SomeEvent.SOME_TYPE).toCommand(SomeCommand); commandMap.map(AnotherEvent.ANOTHER_TYPE).toCommand(SomeCommand);

but you can inject someVO into the command, set to different values before dispatching each event to trigger the command. Not saying that I’d use it like this, mentioning it just as a possibility.

It’s definitively very cool that PayloadEvent circumvents the problem of 2 events triggering the same command.