2

My program allows the server to send a module to a client, and the client will a respond back with module information after it has loaded it. The server store stores the client information in a List called 'loadingModules'. When the server receives that response from the client, it will remove the information from said list to indicate the load operation has completed for that module.

The problem here is is the way I'm currently doing it is by looping through the entire list and checking if two members are equal (ModuleName, and TimeStamp). This is not only just inefficient, but also makes the code look terrible. I am wondering if there is some way to do it using a hash of those two members to access the modules in perhaps a dictionary instead of what I am currently doing.

public List<ClientModule> LoadingClientModules = new();
public List<ClientModule> LoadedClientModules = new();

private void ClientModuleAdded_Callback(Packet packet)
{
    ClientModule f = Utils.ByteArrayToStructure<ClientModule>(packet.Payload, 0);
    Utils.Log.Information($"{_ClientInformation.ComputerUser} Loaded  {f.ModuleName} TimeDateStamp: {f.TimeDateStamp} Address: {f.Address}");
    LoadedClientModules.Add(f);

    LoadingClientModules.RemoveAll(i => i.ModuleName == f.ModuleName && i.TimeDateStamp == f.TimeDateStamp);

    OnClientModuleLoaded?.Invoke(this, f);
}
Meme Machine
  • 949
  • 3
  • 14
  • 28
  • Yes you can. Create a string from a relevant ToString() overload of the two types and use a Dictionary. If you will only have one of each module in it, you could instead just use the module name as the key and then compare the date if you get an object back. – Luke Briner Aug 05 '22 at 14:29
  • The one-liner looks terrible? I dont get the point. Is `LoadingClientModules` really so huge that it's inefficient? Is there only one with ModuleName and TimeDateStamp or do you expect duplicates? – Tim Schmelter Aug 05 '22 at 14:47
  • @TimSchmelter This is just a single tiny example from many in the project. There are many other parts through my project now where this is required where I now have to do several loops due to this. So in my opinion, I would say it makes the code hard to read. There can be multiple ones with the same name, but they can be different versions. (There will never be two with the same name and same version) – Meme Machine Aug 05 '22 at 14:54

2 Answers2

2

You could provide a custom IEqualityComparter<ClientModule> that compares these 2 properties. Then you can use that for a Dictionary or - better here - a HashSet<ClientModule>:

public HashSet<ClientModule> LoadingClientModules = new(new ClientModuleComparer());
public HashSet<ClientModule> LoadedClientModules = new(new ClientModuleComparer());

private void ClientModuleAdded_Callback(Packet packet)
{
    ClientModule f = Utils.ByteArrayToStructure<ClientModule>(packet.Payload, 0);
    LoadedClientModules.Add(f);
    LoadingClientModules.Remove(f);
}

Here's a possible implementation:

public class ClientModuleComparer : IEqualityComparer<ClientModule>
{
    public bool Equals(ClientModule? x, ClientModule? y)
    {
        if (x == null && y == null) return true;
        if (x == null || y == null) return false;
        return x.ModuleName == y.ModuleName && y.TimeDateStamp == y.TimeDateStamp;
    }

    public int GetHashCode([DisallowNull] ClientModule obj)
    {
        unchecked // Overflow is fine, just wrap
        {
            int hash = 17;
            // Suitable nullity checks etc, of course :)
            hash = hash * 23 + obj.ModuleName.GetHashCode();
            hash = hash * 23 + obj.TimeDateStamp.GetHashCode();
            return hash;
        }
    }
}

Note that this does not allow duplicates anymore. So you will not have multiple with the same ModuleName and TimeDateStamp in these collections. Add returns false in case of a duplicate.

Tim Schmelter
  • 450,073
  • 74
  • 686
  • 939
  • Thanks! Would you be able to explain the custom GetHashCode function? Particularly, just the calculations you're making. – Meme Machine Aug 05 '22 at 15:04
  • @MemeMachine: that's just a copy&paste from the best practise answere here: https://stackoverflow.com/questions/263400/what-is-the-best-algorithm-for-overriding-gethashcode – Tim Schmelter Aug 05 '22 at 15:05
0

Using a dictionary to store and find the data is always faster than implementing a list and searching through the list item but it costs more memory.

You may use a Tuple created from ModuleName and TimeDateStamp and use it as the key to define a dictionary:

public Dictionary<(string,DateTime),ClientModule> LoadingModulesDictionary = new ...;

then use the dictionary as:

private void ClientModuleAdded_Callback(Packet packet)
{
    ClientModule f = Utils.ByteArrayToStructure<ClientModule>(packet.Payload, 0);
    LoadedClientModules.Add(f);
    LoadingModulesDictionary.Remove((f.ModuleName,f.DataTimeStam));
}

or define a unique id for each ClientModule object is to add a Guid field to the class and use the guid as the key for dictionary alternatively.

Behnam
  • 337
  • 1
  • 6