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:
- The function takes
connectionSet
(the list of connections to be filtered) andmodifiedContainers
(a list of containers that have been recently updated) as parameters. - 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. - Using LinQ, the connections of
TypeA
are then split in different groups based on the other properties. - 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 the connection is not linked to a container in the
- 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
- Define a
- 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!