0

I am developing a desktop application that will be run at each station at a call center. It listens for an incoming call, and whenever a call arrives, there are about 30 different classes that need to run various operations. Some of these operations are synchronous, such as setting a flag because they want to log only the first time per call that an event happens. Some of them are asynchronous, such as establishing a socket to another application and sending a start message. At the end of the call, the classes will also have to do some clean up, such as sending an end call message, awaiting a response, and then closing the socket.

Currently, I create all the classes in the composition root, and they live for the life of the application. Each time a call arrives, the state of all of these classes gets reset. Here is a simplified, but working, code sample:

    class Program
    {
        static void Main(string[] args)
        {
            var messagePersistence = new MessagePersistence();
            var socketConnection = new SocketConnection(messagePersistence);
            var otherComponent = new OtherComponent();

            ICallComponent[] Components = { messagePersistence, socketConnection, otherComponent };
            TestCall testCall = new TestCall(Components, socketConnection);

            testCall.Execute().Wait();

            // Create another TestCall instance or run the same one again?
            testCall.Execute().Wait();

            Console.WriteLine("Press any key to end");
            Console.ReadKey();
        }
    }

    class TestCall
    {
        private IEnumerable<ICallComponent> _components;
        private IDataReceiver<byte[]> _dataDestination;

        public TestCall(IEnumerable<ICallComponent> components, IDataReceiver<byte[]> dataDestination)
        {
            _components = components;
            _dataDestination = dataDestination;
        }

        public async Task Execute()
        {
            // Start the components
            foreach (ICallComponent Component in _components)
            {
                await Component.StartAsync();
            }

            // This simulates the actual work, which would periodically pull bytes from some source and send them to _dataDestination
            using (var Timer = new System.Timers.Timer(500))
            {
                Timer.Elapsed +=
                    async (sender, e) =>
                    {
                        await _dataDestination.ProcessData(new byte[] { 1, 2, 3 });
                    };
                Timer.Start();

                await Task.Delay(5000);
            }

            // Stop the components
            foreach (ICallComponent Component in _components)
            {
                await Component.StopAsync();
            }
        }

    }

    interface ICallComponent
    {
        Task StartAsync();
        Task StopAsync();
    }

    interface IDataReceiver<T>
    {
        Task ProcessData(T data);
    }

    class SocketConnection : ICallComponent, IDataReceiver<byte[]>
    {
        private IDataReceiver<string> _messageDestination;

        public SocketConnection(IDataReceiver<string> messageDestination)
        {
            _messageDestination = messageDestination;
        }

        public async Task StartAsync()
        {
            Console.WriteLine("Making socket connection");
            await Task.Delay(1);
            Console.WriteLine("Executing initial connection handshake");
            await Task.Delay(1);
            Console.WriteLine("Socket is connected");
        }

        public async Task StopAsync()
        {
            Console.WriteLine("Executing close connection handshake");
            await Task.Delay(1);
            Console.WriteLine("Closing socket connection");
            await Task.Delay(1);
            Console.WriteLine("Socket is closed");
        }

        public async Task ProcessData(byte[] data)
        {
            // Simulate sending data on the socket, and sometimes getting a response that would trigger
            // a string going to the destination
            await _messageDestination.ProcessData("ABC");
        }
    }

    class MessagePersistence : ICallComponent, IDataReceiver<string>
    {
        public async Task StartAsync()
        {
            Console.WriteLine("Creating call database record, start time={0}", DateTime.Now);
            await Task.Delay(1);
            Console.WriteLine("Database record created");
        }

        public async Task StopAsync()
        {
            Console.WriteLine("Finalizing call database record, end time={0}", DateTime.Now);
            await Task.Delay(1);
            Console.WriteLine("Database record finalized");
        }

        public async Task ProcessData(string data)
        {
            Console.WriteLine("Creating message database record, time={0}", DateTime.Now);
            await Task.Delay(1);
        }
    }

    class OtherComponent : ICallComponent
    {
        public async Task StartAsync() { await Task.Delay(1); }
        public async Task StopAsync() { await Task.Delay(1); }
    }

My problem is that this seems kind of fragile. The order in which the objects is started becomes very important, especially if the objects are related. Otherwise you can run into a situation where SocketConnection is feeding messages to MessagePersistence, which hasn't yet completed its startup. This prevents doing something like running the tasks in parallel.

I feel like it would be better to create these objects as needed, so they can never be in an invalid state. In other words, the call started event would call a factory method to asynchronously create objects. That way, the MessagePersistence dependency passed to SocketConnection will be guaranteed to be in a valid state for receiving messages before SocketConnection is even created.

So my thought would be to make an abstract factory like the following:

    interface ICallComponentFactory
    {
        Task<IEnumerable<IAsyncDisposable>> CreateCallComponentsAsync();
    }

    interface IAsyncDisposable
    {
        Task DisposeAsync();
    }

The call started event handler would call the factory method to get a collection of IAsyncDisposables, which it would save in a shared data class injected into it. The call ended event handler would also be injected with this shared data class and could just go through all of them and call DisposeAsync to do the cleanup.

Does this technique make sense? I'm used to a factory creating one instance of an object. Is it weird to have a factory that creates a bunch of objects and returns all of them in a collection? Or is that overkill and things are fine the way they are? Or maybe there is some cleaner way that I'm missing? I'm using C# 6.0 if it matters.

  • "I'm using C# 6.0 if it matters." Why are you using such an antiquated version of the language? C# 8 is in beta and C# 7 is coming up to two years old. v7.0 - v7.3 added some huge improvements to the language. So upgrade as soon as you can. – David Arno May 16 at 7:13
  • @DavidArno I think if you download any current IDE and start a project, it will automatically use the newest stable version. Whatever the reason was, it was probably good enough to warrant the extra effort of specifically setting a lower version (instead of just starting developing as-is). – R. Schmitz May 16 at 11:49
  • Not by any stretch of the imagination an answer, but "Does this technique make sense? I'm used to a factory creating one instance of an object." - in my experience, too, it always felt cleaner to have a factory produce single instances instead of a collection. In your situation, I might create a new class CallComponentSet, very simple, just to hold that bunch of ICallComponents. Then you can still have a clean-seeming CallComponentSetFactory that only creates a single instance of something. – R. Schmitz May 16 at 12:02
  • @DavidArno My company uses Visual Studio 2015 and there are no plans to upgrade. I don't pick the tools - I just use the ones placed before me. – Rob L May 20 at 17:57
3

Just create new objects when you have a new call.

It's pointless to spend hours debugging and tweaking code to slightly more efficiently use 5 cents worth of memory.

  • 3
    Especially since the "re-use objects for efficiency" idea comes from the dark ages of garbage collection, which have been over for more than 30 years. In a modern, high-performance, generational GC, it actually hurts performance, because it breaks the Generational Hypothesis: "Old Objects Do Not Reference New Objects, and Most Objects Die Young, and Most Objects Are Never Mutated". That's the basic assumptions that modern GCs are built on, if you violate those, they become much less performant. – Jörg W Mittag May 16 at 5:05
  • So I've started doing this and now my factory looks like a composition root all on its own. It has knowledge of like 30 other classes, each of which have their own dependencies, some of which are necessarily singletons, since they use resources like speakers and microphone. Is this a case where using a service locator is my only choice? – Rob L May 16 at 21:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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