DecisionProjectionBase on Java version

[fpellet]

Add test to check if event is managed by projection. Currently, an exception of NullPointer type is raised.

[jeantil]

I have looked into this, and I am not sure I want to actually change the way it is done at the moment except maybe throw a specific exception to explain what the issue is as NPE is a bit too generic.

The NPE can only happen if the event wasn't registered which is always an error :

• either register the event
• check whyt the aggregate received an event which it isn't concerned with.

Regarding adding a custom exception I am curious how you would go about it in the scope of the exercise. DecisionProjectionBase is an extraction of the common code in Message#DecisionProjection and Identity#DecisionProjection. This raises a couple questions for me :

• Would you add the exception throwing and corresponding test on the aggregates or would you introduce a specific unit test for DecisionProjectionBase ?
• Would you define a generic exception for not having registered the event or use an aggregate specific exception ?

[jeantil]

Looking at the C# version https://github.com/DevLyon/mixter/commit/a0973e24c3a241b314ced1dede526ec07b12a0c8#diff-69cf78d87e13cf1a1f83cd5c2aece0f7L46

what is the behaviour of

public void Apply(IDomainEvent @event)
{
  When((dynamic)@event);
}

When passed an event for which there are no implementations ? is it silently ignored as is the case in the introduced DecisionProjectionBase or does it fail ?

A quick test using a c# repl

using System;

public interface IBar {}
public class Bar : IBar {}
public class Baz : IBar {}
public sealed class FooBar : Bar {}

// Simple helper for demonstration
public static class ConsolePrinter
{
    public static void Print(Bar item)
    {
        Console.WriteLine("Bar");
    }

    public static void Print(FooBar item)
    {
        Console.WriteLine("FooBar");
    }
}

class MainClass {
  public static void Main (string[] args) {
    var bar = new Bar();
    var baz = new Baz();
    var foo = new FooBar();

    IBar[] items = { bar, foo, baz };
     foreach (dynamic item in items)
    {
        ConsolePrinter.Print(item);
    }
  }
}

yields

Unhandled Exception:
Microsoft.CSharp.RuntimeBinder.RuntimeBinderException: The best overloaded method match for `ConsolePrinter.Print(Bar)' has some invalid arguments
at (wrapper dynamic-method) object.CallSite.Target (System.Runtime.CompilerServices.Closure,System.Runtime.CompilerServices.CallSite,System.Type,object) <0x0017d>
at System.Dynamic.UpdateDelegates.UpdateAndExecuteVoid2<System.Type, object> (System.Runtime.CompilerServices.CallSite,System.Type,object) <0x00500>
at (wrapper dynamic-method) object.CallSite.Target (System.Runtime.CompilerServices.Closure,System.Runtime.CompilerServices.CallSite,System.Type,object) <0x0013c>
at MainClass.Main (string[]) <0x001ce>

Therefore I argue that the refactoring in the C# branch actually changes the behaviour of the system from throwing an exception to silently ignoring unregistered events without introducing a specific test ;p

[fpellet]

:) This implementation change doesn't add any desired behavior, because tests check that a command works well. The internal behavior is only an implementation detail. This issue is just for the intermediate step and help in error analyzing on tests. The problem is just the null pointer. After, ignoring the event or sending a more telling exception, it's ok for me. You may be able to write a test with a false implementation, just to test this creation step.