Hacker News Reader

Comment by 0xcoffee | Hacker News Reader

I didn't really understand the part about 'What Problem Does 'Ref' Solve?'

What would be the downside of writing it such as:

`ExecuteActivityAsync<OneClickBuyWorkflow>(e => e.DoPurchaseAsync, etc...)`?

That's basically what is happening here, except you create `e` ahead of time instead of a do-anything-you-want lambda. But they essentially the same thing, though I think `ExecuteActivityAsync(MyActivities.Ref.DoPurchaseAsync, etc...)` is clearer that you are referencing an activity. And since it's just a delegate that an attribute is on, you can write a helper to do what you have instead.

If you're worried about someone doing something other than calling a method in the lambda it's pretty straightforward to use an Expression<Func<T, TResult>> and validate the contents of the expression. Seems like that would make for a much better dev experience than this static Ref property pattern that you don't see anywhere else in C#.

It's not just that worry, it's the other reasons on top of that detailed in the post. But note that GP's post doesn't _call_ the method, it just references it which is basically what we are doing in the post. But for things like Durable Entities which _do_ call the method, yes we can prevent multi-use of the lambda arg but there are other problems (forcing interface/virtual, can't allow other args, sync vs async, etc).

I'm not really arguing that your pattern _doesn't work_, I just think you've created an unnecessary new pattern to solve an already solved problem. Using expressions with lambdas is pretty standard in C# when you need to reference a method or property without calling it immediately. Entity Framework is an obvious example, mocking libraries like Moq, and at least one that I know of (Hangfire) uses expressions to serialize info about a method so that it can be invoked later, possibly on a different machine or even in a totally different app. All of the things that you mention can be validated in an expression, if you feel the need.

Thanks! Other commenter has also convinced me to investigate expression approach. I was hoping not to force people to create lambdas for each thing they want to invoke from a code readability POV, but it sounds like the ecosystem wants to force that.

The main reason I'm curious is because they write

'We solve this problem by allowing users to create instances of the class/interface without invoking anything on it. For classes this is done via FormatterServices.GetUninitializedObject and for interfaces this is done via Castle Dynamic Proxy. This lets us "reference" (hence the name Ref) methods on the objects without actually instantiating them with side effects. Method calls should never be made on these objects (and most wouldn't work anyways).'

Which sounds like a lot of heavy lifting. It seems something like

    public class WorkflowBuilder<T> where T : class
    {
        // Somehow workflow gets the real instance
        private T Instance;
    
        public async Task<TResult> ExecuteActivityAsync<TResult>(Func<T, Task<TResult>> func) => await func(Instance);
    
        public async Task<TResult> ExecuteActivityAsync<TResult>(Func<Task<TResult>> func) => await func();
    
        public async Task ExecuteActivityAsync(Func<Task> func) => await func();
    
        public async Task ExecuteActivityAsync(Func<T, Task> func) => await func(Instance);
    }
    
    public record Purchase(string ItemID, string UserID);
    
    public class PurchaseActivities
    {
        public static WorkflowBuilder<PurchaseActivities> OneClickBuyWorkflow => new WorkflowBuilder<PurchaseActivities>();
    
        public async Task DoPurchaseAsync(Purchase purchase)
        {
            await OneClickBuyWorkflow.ExecuteActivityAsync(e => e.DoPurchaseAsync(purchase));
        }
    
        public static async Task DoPurchaseAsyncStatic(Purchase purchase)
        {
            await OneClickBuyWorkflow.ExecuteActivityAsync(() => DoPurchaseAsyncStatic(purchase));
        }
    }

Would pretty much achieve the same thing.

> Which sounds like a lot of heavy lifting

How is `e` created for the `e => e.DoPurchaseAsync(purchase)` lambda? You're going to have to do that lifting anyways to create an instance for `e` that isn't really a usable instance. Unless you use source generators which we plan on doing.

I think what you have there is a lot more heavy lifting. Also note that workflows and activities are unrelated to each other. Workflow can invoke any activities. The code you have is a bit confusing because `PurchaseActivities` should be completely unrelated to workflows.

>You're going to have to do that lifting anyways to create an instance for `e` that isn't really a usable instance.

But this should never happen, these is no reason to create an unusable instance. The real instance should be resolved in the same way however the current workflow resolves it.

Activities may actually run on completely different systems than where the workflow calls execute on. All "execute activity" is from a workflow perspective is telling Temporal server to execute an activity with a certain string name and serializable arguments. Everything else is sugar. So if you're gonna use a type caller-side to refer to the name and argument types, you can't instantiate it fully (its constructor may have side effects for when it really runs).

You can jump through a bunch of hoops like requiring interfaces which is what some frameworks do. But in our case, we just decided to make it easy to reference the method without invoking it or its instance.

Lamba/Func/Expressions are doing exactly this in C#, there is no instantiation required. Creating a unusable Ref object is jumping through hoops.

You can parse an expression to serialize it and run it on a different server etc

See e.g. https://github.com/6bee/Remote.Linq

Hrmm, I will investigate using an expression tree for this (now's the time while it is alpha). I was hoping to avoid people having to create lambdas. I hope I don't run into overload ambiguity with the existing `ExecuteActivity` calls where you can just pass an existing method as Func<T, TResult> param. I will investigate this approach, thanks!

Of course the "Ref" pattern is user-choice/suggested-pattern, it's not a requirement in any of our calls that just take simple delegates however you can create those delegates. So I may be able to work it in there.

If it's possible to use both approaches without cluttering your API, that may be the best solution. I know a lot of more junior devs that have a difficult time wrapping their head around expressions in C#. Excellent library!

After looking at it, I am concerned it is not possible for a clean experience. We have to give guidance and samples and we have to choose a way of referencing methods in those. Having ambiguous approaches is a bit rough. We may have to just move to expressions.

I completely understand. Either way, I still think the library is great and appreciate the work done to create it.

As another point of reference, Hangfire also uses expressions for a similar use-case and it seems to work quite well.

E.g. https://docs.hangfire.io/en/latest/background-methods/passin...

   BackgroundJob.Enqueue<EmailSender>(x => x.Send(13, "Hello!"));

Yup!!!

Reply on news.ycombinator.com