-1
\$\begingroup\$

Please see the code below, which I have taken and adapted from here and here (RPCServer and RPCClient):

public RpcClient()
        {
            var factory = new ConnectionFactory() { HostName = "localhost" };

            connection = factory.CreateConnection();
            channel = connection.CreateModel();
            replyQueueName = channel.QueueDeclare().QueueName;
            consumer = new EventingBasicConsumer(channel);
            List<String> responses = new List<string>();
            consumer.Received += (model, ea) =>
            {
                var body = ea.Body;
                var response = Encoding.UTF8.GetString(body);

                responses.Add(response);
                if (responses.Count == 2)
                {
                    if (!callbackMapper.TryRemove(ea.BasicProperties.CorrelationId, out TaskCompletionSource<List<string>> tcs))
                        return;
                    tcs.TrySetResult(responses);
                }
                else
                    return;
            };
        }

This is a Scatter Gather. Notice that the list of responses is defined outside of consumer.Received. Are there any pitfalls doing this that I have not considered?

\$\endgroup\$

closed as off-topic by t3chb0t, Toby Speight, Graipher, yuri, Gerrit0 Mar 25 at 18:21

This question appears to be off-topic. The users who voted to close gave this specific reason:

If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ could you tell us what pitfalls you already considered? Also, some details could be helpfull, like what kind of problems do you have in mind? Bugs? Performance? Memory-Consumption? Readability? \$\endgroup\$ – d_hippo Mar 24 at 18:57
  • \$\begingroup\$ @d_hippo, I cannot think of any pitfalls whatsoever. I am thinking more about bugs, but would also like to hear about performance. \$\endgroup\$ – w0051977 Mar 24 at 19:08
  • \$\begingroup\$ What does it mean you have adapted the code? What is this code for? You have share more details. \$\endgroup\$ – t3chb0t Mar 25 at 8:01
  • \$\begingroup\$ @t3chb0t, I am trying to implement the scatter gather pattern (it is more of a learning exercise). The full code can be found in the links of my question. \$\endgroup\$ – w0051977 Mar 25 at 8:12
2
\$\begingroup\$

What you are doing is called a 'closure', the C# Language Specification speaks of 'Captured outer variables' in §7.15.5.1.

Since I don't know how familiar you are with them, nor what you intend to actually do with them, I'll try to list some points that you should be aware off (this list will not be exhaustive and is not meant to).

  • Closures can both greatly enhance and reduce the readability and understandability of your code, depending on how you use it and your/your teammates preferences. Introducing a new language feature into day-to-day use should be discussed beforehand with all relevant team members; sometimes someone has a hard time getting something and using it in a productive manner. It's important to establish a certain minimal required knowledge about new language features before they are used in production code, otherwise you might end up with avoidable problems (like: the only dev who fully understands feature X leaves and now you get a critical bug in code that makes extensive use of this feature).
  • Performance in general is a tricky question that I'm not going to try to answer here, since there are a bazillion factors contributing. What I can say from my experience as a C#-Dev is that closures never turned out to be the reason for a performance problem for me. If you're generally writing sane code performance-wise (i.e. code that avoids doing outright stupid things, not code that is micro-optimized), you'll most likely not end up with any closure-related performance problems, and worrying about potential problems that you're not going to have is pointless. If you encounter performance problems, follow the usual procedure before jumping to any hasty conclusions about closures.
  • In my experience, closures do not lead to more bugs, if everyone understands how they work they might lead to fewer. They are mostly syntactic sugar, a syntactically more convenient way to do what you would be doing anyways (in this case: make a variable, whose lifetime is not tied to the scope of a method, an instance-variable of some class and expose it to those who need to know) - which is better tested then what you would be doing on your own, and often more precise. The 'if everyone understands how they work'-part is critical, but thats a no-brainer anyway, any dev using a tool he does not understand will produce problems.

Some information about clorsure-related problems I've run into:

  • Closures are implemented via compiler-generated classes that hold the captured variables. At maximum, one such class per Method is generated, no matter how many closures you use in that Method, which might lead to the unexpected prolonging of an objects lifetime.
  • The foreach-loop underwent a breaking change in respect to how it handles closures in C# 5, which might be a Problem if you try to use the same Code with different Versions of C# (for example, upgrade a C#4-Project to a later language version). Note that this can lead to very subtle problems, like an anonymous method being thread-safe in C#5, bus not in C# 4 (with the Code unchanged). If you're using only C# 5 or greater, you should be safe, if you're using C# 4 or below and are planning an upgrade I would not introduce closures right now.

In your posted code I don't see any reasons not to use the closure.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the reference to closure. +1 for that. Can you see any problems using a scatter gather scenario like this? \$\endgroup\$ – w0051977 Mar 24 at 21:03

Not the answer you're looking for? Browse other questions tagged or ask your own question.