dotnet / csharplang

The official repo for the design of the C# programming language
11.43k stars 1.02k forks source link

IEnumerable<In,Out> #1957

Closed NoamWhy closed 5 years ago

NoamWhy commented 5 years ago

Consider the following fictitious code:

void Main()
{
      Communicate_With_Client ( Order_Business_Logic () );
}

void Communicate_With_Client (IEnumerable<string, string> Business_Logic)
{
      var Input    = null as string;
      var Output   = Business_Logic().GetEnumbrator();

      while (Output.MoveNext (Input))
      {
            Console.WriteLine(Outputs.current);

           Input = Console.ReadLine();
      }
}

IEnumerable<string,string> Order_Business_Logic()
{
      //Perform some business logic
      //...

      //Ask a question, and get an answer
      var Input = yield return “What would you like to order?”;

      //Perform some business logic
      //...

      Input = yield return “What color are you interested in?”;

      //Perform some business logic
      //...

      Input = yield return “Your order has been placed, would you like to order additional items?”;

      //Perform some business logic
      //...

      yield break;
}

This code will, unfortunately, not compile for the following reasons:

  1. There is no such type as IEnumerable<In,Out>.
  2. MoveNext() does not accept any input.
  3. Yield return cannot be used in an assignment statement.

These limitations restrict the enumeration process to be one-directional, and make the implementation of such a back-and-forth discussion between client and server more difficult to implement.

Surely, one can think of many ways to feed back information into the enumeration function, but it would be way more elegant if the language could provide you with a gold standard way of implementation.

CyrusNajmabadi commented 5 years ago

This would definitely be an interesting feature. It's also how javascript iterators work. And that's been used to great effect in JS land. For example, TS uses that facility when rewriting async/await so that the result of an 'await' call is actually 'yielded' into the iterator they generate under the covers.

CyrusNajmabadi commented 5 years ago

@NoamWhy Can you actually present your full definition for IEnumerable<TIn, TOut> for completeness? It would also be good to present waht changes you'd like, rather than stating why this does not work today.

HaloFour commented 5 years ago

How would input flow from foreach back into MoveNext(TIn)?

NoamWhy commented 5 years ago

@CyrusNajmabadi

I'm thinking out loud here, so this may not be perfect yet. Here is what I have in mind at the moment:

public interface IEnumerable<TIn, TOut> : IEnumerable
{
        IEnumerator<TIn, TOut> GetEnumerator();
} 
public interface IEnumerator <TIn, TOut>: IEnumerator
{
        TOut Current { get; }

        bool MoveNext(TIn Input=null);
}

@HaloFour

Notice that I have set to null the default input in MoveNext (TIn Input=null). This would allow you to use foreach with a default input of null. I'm not completely happy with this, I think we need to find a way to feed non-trivial input in a foreach loop too.

jnm2 commented 5 years ago

This sounds very similar to what https://github.com/dotnet/csharplang/issues/43 solves.

svick commented 5 years ago

@jnm2 I don't think so, at least not directly. Async streams are basically about making MoveNext async. This proposal is about making MoveNext accept input.

HaloFour commented 5 years ago

@NoamWhy

I don't think it would make sense to implement IEnumerable or IEnumerator. There's no need to carry the baggage of the days of pre-generics. Then you don't have to worry about a default value; make the input required.

@svick

Agreed, although given the use case of client/server communication it would seem that supporting something like this for async streams would be more appropriate.

orthoxerox commented 5 years ago

This looks like a coroutine, doesn't it?

NoamWhy commented 5 years ago

This would allow us to write the following cool code, using Linq:

Business_Logic.Execute(Out => GetInput(Out));

Where we define the extension:

delegate TIn LambdaGetInput<TIn, TOut>    (TOut Output);

void Execute (this IEnumerable<TIn, TOut> Business_Logic, LambdaGetInput Get_Input)
{
       var Enum = Business_Logic.GetEnumerator();

        while (Enum.MoveNext(Get_Input(Enum.current)));
}
CyrusNajmabadi commented 5 years ago

public interface IEnumerable<TIn, TOut> : IEnumerable

this shoudl probably extend IEnumerable<TIn>, no?

NoamWhy commented 5 years ago

@orthoxerox

Sure this is a coroutine, but what makes this suggestion powerful is the ability to have a two-way communication channel, implemented using a coroutine.

CyrusNajmabadi commented 5 years ago

I'm not completely happy with this, I think we need to find a way to feed non-trivial input in a foreach loop too.

I'm not convinced we would. I think bidirectional iterators are very specialized, and would not be used primarily in normal foreachs. I would expect people to be handrolling so they could control things here.

svick commented 5 years ago

I think I would like to see some evidence that:

  1. This is would be commonly useful.

    So far, the only actual use case that was mentioned is implementing async-await and C# doesn't need that.

  2. A library solution (or solutions) would not be sufficient.

    For example, here is a simple implementation of this idea using await (it's nowhere near production ready, but it works for this code and hopefully gets the point across):

    class Program
    {
        static void Main()
        {
            Communicate_With_Client(CreateCommunicator<string, string>(Order_Business_Logic));
        }
    
        delegate bool Communicator<TInput, TOutput>(TInput input, out TOutput output);
    
        static void Communicate_With_Client(Communicator<string, string> Business_Logic)
        {
            var Input = null as string;
    
            while (Business_Logic(Input, out var Output))
            {
                Console.WriteLine(Output);
    
                Input = Console.ReadLine();
            }
        }
    
        static async Task Order_Business_Logic(Func<string, Task<string>> yield)
        {
            //Ask a question, and get an answer
            var Input = await yield("What would you like to order?");
    
            //Perform some business logic
            Console.WriteLine($"The user ordered {Input}.");
    
            Input = await yield("What color are you interested in?");
    
            //Perform some business logic
            Console.WriteLine($"The user wants it in {Input} color.");
    
            Input = await yield("Your order has been placed, would you like to order additional items?");
    
            //Perform some business logic
            Console.WriteLine($"Does the user want additional items? {Input}");
    
            return;
        }
    
        static Communicator<TInput, TOutput> CreateCommunicator<TInput, TOutput>(
            Func<Func<TOutput, Task<TInput>>, Task> communicatorFunction)
        {
            TOutput tmpOutput = default;
            TaskCompletionSource<TInput> inputTcs = null;
            Task result = null;
    
            return (TInput input, out TOutput output) =>
            {
                inputTcs?.SetResult(input);
    
                if (result == null)
                {
                    result = communicatorFunction(o =>
                    {
                        tmpOutput = o;
                        inputTcs = new TaskCompletionSource<TInput>();
                        return inputTcs.Task;
                    });
                }
    
                output = tmpOutput;
    
                return !result.IsCompleted;
            };
        }
    }
NoamWhy commented 5 years ago

I would expect people to be handrolling so they could control things here.

I agree.

public interface IEnumerable<TIn, TOut> : IEnumerable

this should probably implements IEnumerable, no?

No, it should implement IEnumerable< TOut >, which is what we have today.

CyrusNajmabadi commented 5 years ago

No, it should implement IEnumerable< TOut >, which is what we have today.

Yup. Sorry, that's what i meant.

CyrusNajmabadi commented 5 years ago

Here is what I have in mind at the moment:

Note: You can use ```c# in order to get C# coloring of your sample code.

NoamWhy commented 5 years ago

@svick

I understand that "await yield" may be used to implement such a two-way communication, though this would be an inefficient overkill. Let's use await where it is indeed needed.

And by the way, "await yield", is not making any logical sense, since when a co-routine yields, it is by definition entering an await state. So "await yield" is a kind of a logical redundancy.

YairHalberstadt commented 5 years ago

I would be interested in seeing an actual real life problem that would be best solved this way. I feel like that would be necessary in order to evaluate if this feature is necessary, and if so, what form it should take.

NoamWhy commented 5 years ago

@YairHalberstadt

This could greatly simplify implementing complicated server-side workflows which require non trivial dialogs with a client (who may not necessarily be a human). The example I gave of placing an order is a very real-life one. Placing an order may involve non-trivial workflows performing inventory availability checks, offering alternatives, negotiating terms, etc.

YairHalberstadt commented 5 years ago

@NoamWhy I would like to see an actual example in code. All the examples I can think of would actually be better off rewritten in other ways. I'm trying to work out if I'm just being small minded, or this is not actually that useful.

HaloFour commented 5 years ago

That use case doesn't make sense with IEnumerable<...> as it would inherently be asynchronous. A flow using async/await makes much more sense. That's also a use case where the iterator and the enumeration don't exist in the same process.

These are specialized use cases which are not that common and can be solved through libraries using existing language coroutine features. Unless there are some other compelling use cases I don't see this being useful enough to warrant language changes.

YairHalberstadt commented 5 years ago

The example given in your proposal actually looks like it could be written as

public string Order_Business_Logic(string input)
{
// Business logic goes here
return “What color are you interested in?”;
}

...
foreach (var order in orders)
{
    Order_Business_Logic(order);
}
NoamWhy commented 5 years ago

@HaloFour

That use case doesn't make sense with IEnumerable<...> as it would inherently be asynchronous. A flow using async/await makes much more sense. That's also a use case where the iterator and the enumeration don't exist in the same process.

Sure it is asynchronous. All this code is running on the server which is handling a communication session with a remote client. Each time the server receives an input from the client, it simply calls MoveNext(Input), and sends the output to the client.

svick commented 5 years ago

@NoamWhy

I understand that "await yield" may be used to implement such a two-way communication, though this would be an inefficient overkill.

It was just a quickly thrown together example. I'm certain that the efficiency could be improved; in the end it could probably be on par with what this proposal would have.

Let's use await where it is indeed needed.

C# already has await. Let's only add new features when they are indeed needed.

And by the way, "await yield", is not making any logical sense, since when a co-routine yields, it is by definition entering an await state. So "await yield" is a kind of a logical redundancy.

You're nitpicking my naming, which was not the point of the code. Imagine you took my twenty lines of code and worked to improve them to your satisfaction for, say, a week. That includes naming, performance, everything.

Now take that imaginary library and compare it with the (equally imaginary) language feature proposed here. Do you still think the language feature is necessary? Or would that library be sufficient for your needs?

HaloFour commented 5 years ago

@NoamWhy

it simply calls MoveNext(Input), and sends the output to the client.

Which is synchronous. Both sides should be asynchronous.

NoamWhy commented 5 years ago

@svick

Now take that imaginary library and compare it with the (equally imaginary) language feature proposed here. Do you still think the language feature is necessary? Or would that library be sufficient for your needs?

As you said "A library solution would not be sufficient.", we need a way to write:

Input = yield return ...

Inserting a redundant "await" after the equal sign results in a cumbersome code, which would only confuse developers.

NoamWhy commented 5 years ago

@HaloFour

Which is synchronous. Both sides should be asynchronous.

Sure, MoveNext(Input) is also done asynchronously, you simply keep a dictionary which maps between Session_Id and business logic enumerators, and whenever a request arrives from a client, you locate the enumerator using the Session_Id, you call MoveNext(Input), passing it the input from the client, and return the result to the client. Clean, simple, and beautiful.

HaloFour commented 5 years ago

we need a way to write:

There is no need to write any specific syntax. There just needs to be a way to solve the problem. There are ways to solve the problem of bidirectional asynchronous interprocess communication that makes significantly more sense than trying to tack an argument into IEnumerator.MoveNext. As demonstrated this is something that can be accomplished using existing language features with a relatively small amount of code using await.

Inserting a redundant "await" after the equal sign results in a cumbersome code, which would only confuse developers.

Having an equals sign before a yield return would be confusing. Developers today expect an equals sign before an await expression. And given that your use case is inherently asynchronous, an await makes perfect sense.

Sure, MoveNext(Input) is also done asynchronously, you simply keep a dictionary which maps between Session_Id and business logic enumerators, and whenever a request arrives from a client, you locate the enumerator using the Session_Id, you call MoveNext(Input), passing it the input from the client, and return the results to the client. Clean, simple, and beautiful.

MoveNext can't be based on IEnumerable if it's asynchronous. That method returns bool and it will block until the client sends a result. Clearly that's unsuitable for a client/server scenario.

svick commented 5 years ago

@NoamWhy

we need a way to write Input = yield return ... Inserting a redundant "await" after the equal sign results in a cumbersome code, which would only confuse developers.

Well, if you're only going to accept that specific syntax, then there is nothing to discuss.

But I think that with that approach, you're not going to arrive at the best possible solution, just the first solution you came up with.

Are you seriously saying that input = yield return output; is so much better than e.g. input = await SendAndReveive(output); or input = await MoveNext(output); or some other, even better name, that the latter is not even worth considering?

svick commented 5 years ago

@HaloFour

I think the point is that the server code would be roughly:

while (Output.MoveNext(Input))
{
    await Send(Output.Current);

    Input = await Receive();
}

So all communication is asynchronous, even though IEnumerable<TIn, TOut> is still synchronous.

NoamWhy commented 5 years ago

@HaloFour

I think the point is that the server code would be roughly:

while (Output.MoveNext(Input)) { await Send(Output.Current);

Input = await Receive();

} So all communication is asynchronous, even though IEnumerable<TIn, TOut> is still synchronous.

You hit the nail on its head. I love this code!

@svick

If I want to await for a MoveNext(), I can simply write (today):

await new Task(() => Enum.MoveNext());

CyrusNajmabadi commented 5 years ago

Yeah, based on these argument so far, i'm in the camp of "this isn't needed". It was an interesting approach for JS to take given htat it enabled them to have a low level primitive they could build both yield and await on top of. However, given that C# has already crossed that point, it doesn't buy much. The 'async/await' versions seem more idiomatic and cleaner anyways.

await new Task(() => Enum.MoveNext());

Def don't do this. If MoveNext is actually costly, you're killing a thread with blocking.

NoamWhy commented 5 years ago

@svick

I'm certain that the efficiency could be improved...

Why implement a feature that would need to be improved. Let's get it right the first time.

CyrusNajmabadi commented 5 years ago

I feel like the 'right' way to do this in our brave new world is to simply have two IAsyncEnumerables that wrap an input/ouput Pipe to the server you're talking to. The Pipes are the right way to asynchronously send data back/forth, and the wrappers are the parts of your code that deal with rich .net objects and marshal to/from the bytes to talk to the server with.

This could of course all be handled by a library for you. However, it would def give the right programming model that would be extremely easy to use.

CyrusNajmabadi commented 5 years ago

Also, @NoamWhy don't be discouraged if people don't necessarily like your suggestion. It's totally normal, and you will definitely get pushback if people think there is already a suitable way to solve the problem your proposal is attempting to address.

To use a positively ancient adage at this point: all language features start off automatically 'hugely negative' in terms of how they are assessed. Just making any changes to the language is staggeringly costly. Think about it as several dozen people working a very long time, just to design/implement things. And that doesn't cover things like documentation, teaching, impact to the community, etc. etc.

So, to take a feature, the feature must deliver a huge amount of value to offset that cost. And it can't just be even. If a feature's value is equal to the cost, then whybother at all? And why both if it's just marginally better? There is fixed resources here, so you want stuff that is majorly better.

So, often times, people assess with that hat on. If your feature isn't majorly better, the perspective will be: it's not good enough, not worth doing. It's not personal, it's just the hard reality of costs and project management :)

Cheers, and keep the proposals coming!

CyrusNajmabadi commented 5 years ago

You may also want to consider joining gitter.im/dotnet/csharplang and gitter.im/dotnet/roslyn. These are two chatrooms that are more casual and more amenable to just discussing potential ideas for language features prior to making an proposal.

jnm2 commented 5 years ago

(https://gitter.im/dotnet/csharplang)

CyrusNajmabadi commented 5 years ago

Oh interesting. It shows as:

image

For me, i didn't realize it was under anything :)

NoamWhy commented 5 years ago

@CyrusNajmabadi

Think about it as several dozen people working a very long time...

That is perfectly understood, and I may add a big "thank you for a wonderful language which I greatly love".

Having said that, you should be very careful not to confuse between "it's not a very good idea", and "we can't allocate resources to that now". Sometimes there would be a tendency to dismiss good ideas simply because there are only so much ideas you can handle at a time. Perhaps a better approach would be to acknowledge the value of all these ideas by posting them to some (ever growing) public wish-list, rather than dismissing with them for good.

YairHalberstadt commented 5 years ago

@NoamWhy The issues are the wish list

CyrusNajmabadi commented 5 years ago

". Sometimes there would be a tendency to dismiss good ideas

You are presuming it's a good idea. But people have pointed out very good reasons for why it isn't. Most specifically, because it seems highly redundant with functionality that can be done today in a non-hacky manner in the language. Why build a totally new system to replicate functionality that already exists?

This goes into my point about: the feature must deliver a huge amount

That isn't the case here. There is very little marginal value delivered, both in terms of language expressiveness, or in terms of problems that people can solve in a better fashion.

--

As an aside, it's super critical when proposing a language feature to listen to the critiques and understand the alternative existing approaches that are presented. It really helps to understand why either:

  1. the proposal's value is marginal
  2. the proposal's value is huge, but examples are not being presented to demonstrate it

Again, the onus is on those supporting the proposal to present the cases and arguments. And "it's a good idea" is not an argument. To be a good idea it has to really be able to stand on its own merits, esp. demonstrating exactly how it improves things for users and why it's truly substantively better than what we have now.

CyrusNajmabadi commented 5 years ago

@NoamWhy The issues are the wish list

indeed.

Perhaps a better approach would be to acknowledge the value of all these ideas by posting them to some (ever growing) public wish-list, rather than dismissing with them for good.

Proposals won't ever go anywhere if they cannot convince people of their value. You're welcome to file these, but I imagine your end goal would be to get them accepted. For that to happen, significant and substantive demonstrations of value need to be presented, and worthwhile counter-arguments must be present to address the critiques being given.

HaloFour commented 5 years ago

@NoamWhy

Sometimes there would be a tendency to dismiss good ideas

Also note that even for ideas that are generally considered "good" to the extent that the team is actively working on them, the turnaround time is rarely very short. Many of the features being planned for C# 8.0 were proposed in some form before C# 6.0 was released. The costs are enormous and the team is very aware that every change needs to be supported forever. That's why it's such a high bar and any proposal is met with initial skepticism.

DavidArno commented 5 years ago

I'm guessing I'm missing something here as what the OP appears to be asking for is already supported by the language and IEnumerable<T>:

void Main()
{
      CommunicateWithClient(OrderBusinessLogic());
}

void CommunicateWithClient(IEnumerable<(string, Action<string>)> businessLogic)
{
    foreach (var (output, getResponse) in businessLogic)
    {
        Console.WriteLine(output);
        getResponse(Console.ReadLine());
    }
}

IEnumerable<(string, Action<string>)> OrderBusinessLogic()
{
    string input = null;
    void HandleResponse(string response) => input = response;

    //Perform some business logic

    //Ask a question, and get an answer
    yield return ("What would you like to order?", HandleResponse);

    //Perform some business logic

    yield return ("What color are you interested in?", HandleResponse);

    //Perform some business logic

    yield return ("Your order has been placed, would you like to order additional items?", 
                  HandleResponse);

    //Perform some business logic

    yield break;
}
NoamWhy commented 5 years ago

@DavidArno

Thanks David, but I believe I have already figured out a much simpler way to accomplish that:

void Main()
{
      CommunicateWithClient(OrderBusinessLogic());
}

void CommunicateWithClient(IEnumerable<Message> businessLogic)
{
    foreach (var LastMessage in businessLogic)
    {
        Console.WriteLine (LastMessage.Output);
        LastMessage.Input = Console.ReadLine();
    }
}

IEnumerable<Message> OrderBusinessLogic()
{
    var LastMessage = new Message();

    //Perform some business logic

    //Ask a question, and get an answer
    LastMessage.Output = "What would you like to order?";  yield return LastMessage;

    //Look at LastMessage.Input, and perform some business logic
    LastMessage.Output = "What color are you interested in?"; yield return LastMessage;

    //Look at LastMessage.Input, and perform some business logic
    LastMessage.Output = "Your order has been placed, would you like to order additional items?";
    yield return LastMessage;

    //Perform some business logic

    yield break;
}

class Message 
{
    public string Output;
    public string Input;
}
DavidArno commented 5 years ago

@NoamWhy,

Yep, that approach would work too. Would you mind closing this issue therefore as it's no longer needed?

quinmars commented 5 years ago

I know, that issue is already closed. But just for reference, there is an interesting document of Bart de Smet discussing a potential bidirectional iterator implementation for C#:

https://github.com/bartdesmet/AsyncIterators/blob/master/Thoughts%20on%20bidirectional%20iterators.MD

ncthbrt commented 5 years ago

No one has bought up F# computation expressions and monads in this discussion. Having a return value for an enumerable allows for a quite succint/trivial equivalent of F#'s language feature. For example usage of a maybe workflow:

Maybe (() => {
    var someResult = yield return Computation ();
    return Computation (someResult);
})

Here the maybe interpreter could yield early if any of the yielded values are null.

HaloFour commented 5 years ago

@ncthbrt

I think the same argument applies, you'd have to demonstrate what use cases that proposed language feature might solve. You could implement such a coroutine using await also which wouldn't require rearchitecting yield.

And if we were going to go down that path I think I'd rather see language support for do-notation or for-comprehension.

ncthbrt commented 5 years ago

I'd love to see do in C#. Though would presume it's a long shot for language inclusion.

I fail to see how you'd implement a mondic effects system using await. Perhaps my understanding of await is poor outside of the context of a Task<>, but wouldn't someResult here not return early if that value was null using await, as Maybe doesn't have the ability to perform a bind/return.

Use cases this proposed language feature might solve are the same ones which justified the let! syntax in F#. The way I see it, the following constructs are roughly analogous:

F# C#
do! X `yield return X;
return! X yield break; (Not quite the same, no return value allowed)
let! x = X; var x = yield return X;

An example of how JavasScript users have been able to leverage this construct to create do is found here: https://github.com/pelotom/burrido