Framework Discussion

[prankard]

I thought I would create a generic issue to do with the overall productivity of the framework.
Please feel free to jump in and make comments.

I didn't feel like flagging lots of little issues due to the fact that some of them are large feature requests

I come from a AS3 background. And have been making the C# transition for the past few years. I've been using the Unity3D platform (of course, your framework works for this as well).

Overall impressions

Firstly, the code. Your code is very readable, very minimal and every piece provided is very useful. It's exactly what the original idea behind Robotlegs, a minimal useful framework. So kudos!

I've been using another framework for Unity (Strange IOC), which is another Robotlegs framework.
Their API looks very clean and usable. However the internals are no where near as clean and in some places are very unoptimised. It also has some good features, however it also has a lot of features which makes it larger and bulkier.

Which leads me to my next few comments, which are features and comments I found lacking at the moment in your framework (I know it's only alpha etc).

Features / Nitpicking / Help

Inject by name

At the moment, you can't map instance to singleton by name. I can see this as an issue in the future. Having to tag everything by an interface, when we could have a key. I can see this is something restricted by TinyIOC.

Small invoker documentation

So I watched all your videos, then a few days later I started programming. So I kind of forgot some of the details. I then made a context and then tried to execute this:

StartupInvoker startupInvoker = new StartupInvoker();
startupInvoker.Invoke();

Which doesn't error, and then after re-watching parts of your tutorial. I realised it was the DI class doing a bit of work connecting things together.
So it would be good to have the line:

StartupInvoker startupInvoker = DI.Get<StartupInvoker>();
startupInvoker.Invoke();

Or an example of it injecting and then executing in the documentation:

[Inject]
public StartupInvoker startupInvoker;

public void ExecuteStartup()
{
    startupInvoker.Invoke();
}

Mediator Constructor

At the moment when my mediator fires up, the code is as follows:

public TestView view;

TestMediator(IMediatorTarget target) : base(target)
{
    view = target as TestView;
    Debug.Log("Registered target for mediator");
}

It would be really nice, if you injected the view as a condition in the mediator map and did it all behind the scenes. The old robotlegs 1 code, prior to registering the mediator, would make a rule of injecting that view. Then after instantiating would un-apply the injection rule.
So the code could be as follows:

[Inject]
public TestView view;

TestMediator()
{

}

Which would be much cleaner. Furthermore, if this did happen it would be nice for the user to optionally cast still. So if you extend your own mediators you can do this:

AbstractView view;

TestMediator()
{

}

Register()
{
    view = viewComponent as AbstractView;
}

So that would mean adding:
IMediatorTarget viewComponent, into the Mediator. And assigning the value before registration.

Invoker Command (AddContextListener)

To be honest, I'm not sure about this point. It would be great if you were to release the VMIC video to see how it really works.

So the first test I did in MonkeyArms was this:
View to submit add score. To be saved into the model. And then to be updated throughout the application. There are quite a few variations to do this, but my most common one is to do this:
View to Mediator. Mediator to Command (via event/invoker).

Command to save to model.
Then command to dispatch UpdateScoreInvoker.

Mediator to hear from Event Bus to then tell the view the Score.

But I was struggling on that last step to hear from the global event bus from my mediator. What I was doing was this:

[Inject]
public ScoreUpdatedInvoker scoreUpdatedInvoker;

public override void Register ()
{
    UnityEngine.Debug.Log("Registered TotalScoreMediator");
    scoreUpdatedInvoker.Invoked += OnUpdateScore;
}


public override void Unregister ()
{
    UnityEngine.Debug.Log("UnRegistered TotalScoreMediator");
    scoreUpdatedInvoker.Invoked -= OnUpdateScore;
}

public void OnUpdateScore(object sender, EventArgs e)
{
    UnityEngine.Debug.Log("OnUpdateScore");
    view.score = scoreUpdatedInvoker.score;
}

But nothing seemed to be heard.

Injection of Invoker in Command

At the moment, you have to call DI.Get in order to get an invoker that will be tied with MonkeyArms's commands. It would be nice in the future to have an injection rule in Commands to do this:

public class AddScoreCommand : Command
{
    [Inject]
    public IAppModel model;

    [Inject]
    public AddScoreInvoker addScoreInvoker;

    #region implemented abstract members of Command

    public override void Execute ()
    {
        model.SetScore(model.Score + addScoreInvoker.score);
    }

    #endregion
}

Where AddScoreInvoker is an injection rule, that is only applied for this command before execution. So it will be the exact AddScoreInvoker that was fired elsewhere in the application.

To do this obviously. You would need to be able to do this:

AddScoreInvoker addScoreInvoker = new AddScoreInvoker();
addScoreInvoker.score = 5;
addScoreInvoker.Invoke();

Without calling DI.Get(); As DI would make a new rule for the specific case for the command the invoker is attached to.

No multi context

This is just a minor point. Personally I've only ever tried to use a multi context application app once. And then we decided not do in the end. So I don't see it being an important feature. But it's good to note I don't think it's possible to do it yet due to the static DI.
I wouldn't worry about this, but I'd thought I'd mention it (as other users might want it).

Summary

First thanks for writing this framework, I hope this issue doesn't come across rude or pedantic. I just wanted to spend some time offering feedback as you clearly spent quite a lot of time writing this framework.
The biggest issue for me at the moment is not being able to AddContextListener, I'm assuming it is probably possible but my ignorance is preventing me from doing it, it would be great to know how.

I hope your framework takes off. Your videos and approach to using different design patterns with the same framework is very interesting to watch

[benbishop]

Hi James,
Thank you so much for your feedback. I've been dying to get some sort of input from the C# community. I've been really surprised by the overall "meh" attitude about anything other than MVVM from the .NET community when it comes to non web apps. Much like yourself, my background and inspiration comes from 9 years of Flash programming, and I guess I'm used to more debate and/or creativity.

Anyways, I had some time yesterday and I implemented some of your suggestions and pushed each of them to their own branch in Github (please let me know if you have any troubles accessing them.)

The branches are as follows:

MapSingletonInstanceByName

I added the method to DI that lets you map a singleton based on a string name. I also updated the Inject attribute to have an optional Name property. If you take a look at the test class DITests lines 129-156 you can see this in action. I've set this functionality up to where you can use the same Name for different data types. For example I could map a string named "MyNamedSingleton" and also map a name to an int called "MyNamedSingleton" and they won't overwrite each other. I figure since C# uses strict data typing this would be a nice feature. If C# wasn't this would obviously cause all heck to break loose. You can also get an instance via the Get("myName") method of DI.

I think this could be a neat feature in that I could potentially use this to map values like localized button names:

[Inject(Name="login")]
public string LoginButtonLabel;

SetMediatorTarget

I also took a stab at moving away from passing the Mediator Target via constructor. BTW, I use the terminology "target" because mediators can mediate more than just views. They can mediate processes like GPS tracking, Timers, etc..

If you take a look at the TestMediator in MediatorTests (lines 118-149) you will see that the Mediator base class now has a property called Target that you can reference and cast to your will.

ForceSingleton

This branch isn't a direct request from you, but I think it could help solve some issues I think you (and even I) may be running into. If I read your email correctly, I think part of the problem you are experiencing with Invokers is that the invoker you are trying to use to message from a Command to a Mediator is not mapped as a Singleton. If this is the case, the mediator and the command each have their own instance of the invoker type instead of referencing the same one. The way to resolve this:

DI.MapSingleton< ScoreUpdatedInvoker>();

Anytime you map an Invoker to a Command via DI.MapCommandToInvoker(), MA automatically makes that Invoker a singleton. However, if an Invoker is not directly mapped to a command it will be treated as a multi-instance. So in your case, the invoker that starts the command is a Singleton, but the notification invoker that command dispatches is not.

To help with this kind of oversight, I've added an attribute called MustBeSingleton that you can decorate any class with. This kind of acts as a contract and helps express the author's intent. Anytime, MA/DI do a Get or Injection involving a class decorated with this attribute it will throw an exception if that class has not been mapped as a Singleton.

Granted, I could potentially have DI auto map these decorated classes as Singletons, but I think that could open a whole new can of worms.

Other Invoker Issues

I was looking at your request about adding a reference to the Invoker that initiated a command. I think something that is missing from your code is that the command receives an InvokerArgs object in it's Execute method. These InvokerArgs act as a payload of data:

public class AddScoreCommand : Command
{
        [Inject]
        public IAppModel model;



        #region implemented abstract members of Command

        public override void Execute (InvokerArgs args)
        {
            model.SetScore(model.Score + (addScoreInvoker as AddScoreInvokerArgs).score);
        }


}

To clarify how you would dispatch this invoker:

DI.Get<AddScoreInvoker>().Invoke(new AddScoreInvokerArgs(123));

AddScoreInvokerArgs would be something like this:

public class AddScoreInvokerArgs:InvokerArgs{

        public readonly int Score;

        public AddScoreInvokerArgs(int score){
                   Score = score;
        }

}

I could potentially have DI set an InvokerArgs property on the Base Command class much like we now set the Target property of a Mediator per your suggestion. My only hesitancy to this is that right now with the args being passed in the execute method it kind of makes unit testing easier, but I'm not sure if that's a big enough reason to not do it.

Overall, Invokers are more like Robert Penner's AS3 Signals than your traditional AS3 events. If we went with the idea of DI setting an args property on a command it would be more like Joel Hooks' implementation of the AS3 Signal/Command Map.

Please let me know if there's anymore I can clarify for you. I know the documentation is a bit lacking. Hopefully, I can find some time or someone to help with it.

Also, don't hesitate if you have any other ideas or feedback on the API design. I know I've personally pulled just the parts of RL that I used, but that there might be others that other devs can't live without.

I've been only using MA for Android and iOS development, so I would love to hear how this working out for you with Unity.