Replace static codegen'd factory classes with generic factories

[ReubenBond]

Currently, we are exposing codegen'd factory classes for IGrainObservers (there may be others, please add them here).

We should create a generic factory which provides access to the codegen'd factories and hide the concrete implementation.

This will help us with Dependency Injection and give us the flexibility to use runtime codegen (eg, via Roslyn)

[sergeybykov]

@ReubenBond - Do you mean to add GrainFactory.CreateObjectReference()?

[ReubenBond]

I was thinking of "ObserverFactory" or "ClientFactory" but yes, pretty much.

-----Original Message----- From: "Sergey Bykov" notifications@github.com Sent: ‎02/‎02/‎2015 06:40 To: "dotnet/orleans" orleans@noreply.github.com Cc: "Reuben Bond" reuben.bond@gmail.com Subject: Re: [orleans] Replace static codegen'd factory classes with genericfactories (#59)

@ReubenBond - Do you mean to add GrainFactory.CreateObjectReference()? — Reply to this email directly or view it on GitHub.=

[sergeybykov]

My personal preference would be to stick to one static factory class, GrainFactory, for better discoverability.

[ReubenBond]

The proposal is to add

public static Task<TGrainObserverInterface> GrainFactory.CreateObjectReference<TGrainObserverInterface>(TGrainObserverInterFace obj);

As well as the matching DeleteObjectReference method.

LGTYou?

[ReubenBond]

What should the behavior be when a user calls GrainFactory.CreateObjectReference(observer) where observer is a concrete type (i.e, not an interface).

Currently it will crash at runtime when it is unable to find the concrete type's factory.

I think the best approach is to force the user to specify the interface in the type parameters, which equates to changing the signature to:

CreateObjectReference<TGrainObserverInterface>(IGrainObserver obj)
    : where TGrainObserverInterface : IGrainObserver

Usage then becomes:

var obj = await GrainFactory.CreateObjectReference<ISimpleGrainObserver>(observer);

I'm performing a runtime check that the argument matches the type parameter:

if (!interfaceType.IsInstanceOfType(obj))
{
    throw new ArgumentException(
        string.Format("The provided object must implement '{0}'.", interfaceType.FullName),
        "obj");
}

What do you think?

[jkonecki]

Do you really need to perform the check at runtime? The compiler won't allow passing of the object that violates generic parameter restrictions...

On Sun, 1 Feb 2015 22:41 Reuben Bond notifications@github.com wrote:

What should the behavior be when a user calls GrainFactory.CreateObjectReference(observer) where observer is a concrete type (i.e, not an interface).

Currently it will crash at runtime when it is unable to find the concrete type's factory.

I think the best approach is to force the user to specify the interface in the type parameters, which equates to changing the signature to:

CreateObjectReference(IGrainObserver obj) : where TGrainObserverInterface : IGrainObserver

Usage then becomes:

var obj = await GrainFactory.CreateObjectReference(observer);

I'm performing a runtime check that the argument matches the type parameter:

if (!obj.GetType().IsAssignableFrom(interfaceType)) { throw new ArgumentException( string.Format("The provided object must implement '{0}'.", interfaceType.FullName), "obj"); }

What do you think?

— Reply to this email directly or view it on GitHub https://github.com/dotnet/orleans/issues/59#issuecomment-72388970.

[ReubenBond]

I can't have generic parameter restrictions to say T is an interface, though. The compiler will infer the most specific type possible, which will be the concrete type.

To make it less error prone, I changed the type in the parameter list to IGrainObserver rather than TGrainObserverInterface - that forces the consumer to specify the interface type (which I can only check at runtime).

Maybe I'm missing something, though. Do you have a suggestion for how I can improve this?

[veikkoeeva]

Just an idea, would IsAssignableFrom be an option here?

• http://www.hanselman.com/blog/DoesATypeImplementAnInterface.aspx
• http://stackoverflow.com/questions/12160460/when-is-obj-gettype-isinstanceoftypetypeofmyclass-true

[sergeybykov]

The compile will already enforce where TGrainObserverInterface : IGrainObserver

At run time, don't we just need to check that it's an interface via obj.GetType().IsInterface ?

[ReubenBond]

We do a couple of sanity checks:

1. Is TGrainObserverInterface an interface: the compiler enforces that it implements IGrainObserver as you said, @sergeybykov.
2. Is obj an instance of TGrainObserverInterface: the compiler only check that it implements IGrainObserver, but not its relationship to TGrainObserverInterface.

The type system cannot infer the interface from the concrete type, so the user must explicitly provide it. We cannot specify a relationship between obj and TGrainObserverInterface while forcing the user to specify TGrainObserverInterface unless we also force them to explicitly specify the type of obj.

i.e:

// Bad: TInterface is concrete type
Task<TInterface> CreateObjectReference<TInterface>(TInterface obj) where TInterface : IGrainObserver

// Correct, but the user has to specify both types explicitly
Task<TInterface> CreateObjectReference<TInterface,TConcrete>(TConcrete obj) where TInterface : IGrainObserver where TConcrete : TInterface

// Only runtime checks can verify relationship between `obj` and TInterface
Task<TInterface> CreateObjectReference<TInterface>(IGrainObserver obj) where TInterface : IGrainObserver