patterncsharpMinor
Mapping .csv rows to a Dictionary
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
.csv format:
Server class:
Mapping:
Fetching:
Concerns:
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 | application2Server 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
Dictionarythe 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.
This is a very inconvenient way of adding new items to your dictionary because you are accessing it twice for each new item. With
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
Retrieving an item with
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
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:
Now the keys is case-insensitive also during adding.
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.