1
\$\begingroup\$

Hello fellow programmers, I am currently implementing a function and wanted to know if there was anything I could change to simplify the code, make it more readable, or to optimize the running time and memory usage. My current implementation works as expected, without any bugs.

The function I am implementing is called FilterConnectionSet() and filters a given set of connections based on some criteria, and returns a list containing only the connections that satisfy those criteria.

Each connection is linked to what we call a container, which is basically an external tool that can be attached to our software. Each containers has a given type (A, B, C, etc.). When a container is changed/updated, there is a variable named modifiedContainers of type ContainerDictionary that is automatically updated to include the modified container. Note that this variable is passed as a parameter to my function. I cannot change the code that updates the list of modified containers. All I can say is that ContainerDictionary is basically a custom class that wraps a Dictionary<>.

Before performing any container related operation in our software, we perform a "refresh" by importing all the latest data from all the attached containers. However, for performance purposes, we do not want to import data from containers that were not updated since the last refresh. Additionally, even if we do not support that feature yet, multiple containers of a specific type (let's call it TypeA) type can be attached to our software at the same time. In that case, we want to group those containers based on some other properties, and only import the data of a single container per group, and display a warning message to the user that the data of the other containers of the same group were discarded.

Please note that I can only change the function that filters the connections. I cannot make any changes to the functions that takes care of updating the modifiedContainers list, or the actual import of data, or anything else not shown in this questions.

Here's how my FilterConnectionSet function works:

  1. The function takes connectionSet (the list of connections to be filtered) and modifiedContainers (a list of containers that have been recently updated) as parameters.
  2. It then loops through all the connections and and separates all the connections to containers of TypeA. All remaining connections are automatically added to the list of filtered connections.
  3. Using LinQ, the connections of TypeA are then split in different groups based on the other properties.
  4. The function loops through each groups and does the following:
    • Define a connectionToAdd variable, and initialize to last connection in the group.
    • Loop through all the connection in the group
      • IF the connection is not linked to a container in the modifiedContainers list, THEN go to the next connection in the group.
      • ELSE, update the connectionToAdd variable.
    • IF there was more than one connection in the group, THEN display the message stating the only one container data will be imported.
    • Add the connection stored inside the connectionToAdd variable to the list of filtered connections.
    • Go to the next group of connection and repeat
  5. Return the list of filtered connections

Here is my current implementation:

    private static List<Connection> FilterConnectionSet(List<Connection> connectionSet, ContainerDictionary modifiedContainers)
    {
        var filteredConnectionSet = new List<Connection>();
        var typeAConnections = new List<Connection>();
        
        foreach (var connection in connectionSet)
        {
            // Check connection Type. If TypeA, add to list of TypeA connections, else add to filtered set. 
            if (connection.Type == "TypeA")
            {
                typeAConnections.Add(connection);
            }
            else
            {
                filteredConnectionSet.Add(connection);
            }
        }

        // Group the connections of TypeA based on other properties
        var groupedTypeAConnections = typeAConnections.GroupBy(c => new {c.property1, c.property2});

        foreach (var connectionGroup in groupedTypeAConnections)
        {
            var connectionToAdd = connectionGroup.Last();

            foreach (var connection in connectionGroup)
            {
                // Please note here the the ContainerInModifiedList function is a helper function that cannot be changed.
                if (!ContainerInModifiedList(connection, modifiedContainers))
                {
                    continue;
                }

                connectionToAdd = connection;
            }

            // If there is more than one connection in the group, display warning message. Please not that the helper function for displaying messages cannot be changed.
            if (connectionGroup.Count() > 1)
            {
                var warningMessage = string.Format("Multiple containers with property1 = {0} and property2 = {1} were detected. Only the container {2} will be imported!",
                    connectionToAdd.property1,
                    connectionToAdd.property2,
                    connectionToAdd.Container.Name);

                ErrorHelper.PublishWarningMessage(warningMessage);
            }

            filteredConnectionSet.Add(connectionToAdd);
        }

        return filteredConnectionSet;
    }

I hope I provided enough details.

I am open to any comments and suggestion!

\$\endgroup\$
1
  • 1
    \$\begingroup\$ @aepot Anonymous types have structural comparison by default. \$\endgroup\$ Mar 30, 2021 at 20:39

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Browse other questions tagged or ask your own question.