HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Mapping .csv rows to a Dictionary

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mappingcsvrowsdictionary

Problem

I have a .csv file in which each row represents an association between a software application and a web server. My goal is to fetch a List by application, so I'm mapping the .csv to a Dictionary>. I'm using CsvHelper to read the .csv file and map it to objects.

.csv format:

Family | Environment | Name   | Application
01     | Dev         | WEBD01 | application1
01     | Production  | WEBP01 | application1
02     | Dev         | WEBD02 | application2


Server class:

public class Server
{
    public string Name { get; set; }
    public string Family { get; set; }
    public string Environment { get; set; }
}


Mapping:

protected override void RefreshData()
{        
    // _serverDictionary is a class-level Dictionary>
    _serverDictionary.Clear();

    using (TextReader textReader = File.OpenText(_csvFile.FullName))
    using (CsvReader reader = new CsvReader(textReader))
    {    
        while (reader.Read())
        {
            string applicationName = reader.GetField("Application");
            Server server = reader.GetRecord();

            if (_serverDictionary.ContainsKey(applicationName))
                _serverDictionary[applicationName].Add(server);
            else
                _serverDictionary.Add(applicationName, new List { server });
        }
    }
}


Fetching:

public IEnumerable GetByApplication(string applicationName)
{
    List servers = new List();

    _appServers.Where(pair => pair.Key.EqualsIgnoreCase(applicationName))
        .ForEach(pair => servers.AddRange(pair.Value));

    return servers;
}


Concerns:

  • Is a Dictionary the best data structure to use in the first place?



  • I feel that both the mapping code and the fetch code could be more efficient by better utilizing LINQ projection.

Solution

I feel that both the mapping code and the fetch code could be more efficient by better utilizing LINQ projection.

more efficient yes but this time without linq.

if (serverDictionary.ContainsKey(applicationName))
    serverDictionary[applicationName].Add(server);
else
    serverDictionary.Add(applicationName, new List { server });


This is a very inconvenient way of adding new items to your dictionary because you are accessing it twice for each new item. With TryGetValue you can do the same with only one access to it. This means that if you could retrieve the value then you already have and it's ready to use, there's no need to use the indexer anymore. If however it's not there, then you just add it.

if (serverDictionary.TryGetValue(applicationName, out List servers))
{
    servers.Add(server);
}
else 
{
    serverDictionary[applicationName] = new List { server };
}


_appServers.Where(pair => pair.Key.EqualsIgnoreCase(applicationName))
    .ForEach(pair => servers.AddRange(pair.Value));


This might be super inefficient. If you use a dictionary then don't use linq if you don't have to. For fetching you can use the TryGetValue method agian:

return
    serverDictionary.TryGetValue(applicationName, out List servers) 
        ? servers
        : Enumerable.Empty();


Retrieving an item with TryGetValue means it's an O(1) operation whereas the same done with the original query means that the entire dictionary needs to be searched which means O(n). If you need to return a copy of the server list the you might use linq for it and call .ToList() on the servers.

There is one more thing about the linq query. You search for the key by ignoring the case. If you have multiple names but with different case then the resulting list will contain items of all those keys. Is this really what you want? The Where won't stop at the first match.

I don't know how you create the dictionary but if you want to ignore the case of the key you should initialize the dictionary appropriately by specifying the key comparer:

serverDictionary = new Dictionary>(StringComparer.OrdinalIgnoreCase);


Now the keys is case-insensitive also during adding.

Code Snippets

if (serverDictionary.ContainsKey(applicationName))
    serverDictionary[applicationName].Add(server);
else
    serverDictionary.Add(applicationName, new List<Server> { server });
if (serverDictionary.TryGetValue(applicationName, out List<Server> servers))
{
    servers.Add(server);
}
else 
{
    serverDictionary[applicationName] = new List<Server> { server };
}
_appServers.Where(pair => pair.Key.EqualsIgnoreCase(applicationName))
    .ForEach(pair => servers.AddRange(pair.Value));
return
    serverDictionary.TryGetValue(applicationName, out List<Server> servers) 
        ? servers
        : Enumerable.Empty<Server>();
serverDictionary = new Dictionary<string, List<Server>>(StringComparer.OrdinalIgnoreCase);

Context

StackExchange Code Review Q#162057, answer score: 4

Revisions (0)

No revisions yet.