Mapping .csv rows to a Dictionary

Question

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<Server> by application, so I'm mapping the .csv to a Dictionary<string, List<Server>>. 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<string, List<Server>>
 _serverDictionary.Clear();
 using (TextReader textReader = File.OpenText(_csvFile.FullName))
 using (CsvReader reader = new CsvReader(textReader))
 { 
 while (reader.Read())
 {
 string applicationName = reader.GetField<string>("Application");
 Server server = reader.GetRecord<Server>();
 if (_serverDictionary.ContainsKey(applicationName))
 _serverDictionary[applicationName].Add(server);
 else
 _serverDictionary.Add(applicationName, new List<Server> { server });
 }
 }
}

Fetching:

public IEnumerable<Server> GetByApplication(string applicationName)
{
 List<Server> servers = new List<Server>();
 _appServers.Where(pair => pair.Key.EqualsIgnoreCase(applicationName))
 .ForEach(pair => servers.AddRange(pair.Value));
 return servers;
}

Concerns:

1. Is a Dictionary the best data structure to use in the first place?
2. I feel that both the mapping code and the fetch code could be more efficient by better utilizing LINQ projection.

• t3chb0t: serverDictionary is local to the RefreshData method so it actually does nothing. Is this really what you have?
• Tyler Daniels: You're right. I mistakenly tried to adjust my code for the question. I've corrected it to reflect what it's actually doing which is updating a class-level Dictionary.
• paparazzo: If you are going to use Linq like that then you don't even need Dictionary. I am not getting the AddRange? This would be so much less code with just add Application to Server and use a List with Linq. Server does not know Application and I bet that would be useful.
• Tyler Daniels: At one time, I was trying to use IEnumerable<KeyValuePair<T1,T2>> to accommodate a generic interface. Once I moved to a proper dictionary, my mind must have been stuck on using LINQ with the IEnumerable. Sad moment for my brain.

Answer 1

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> { 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<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));

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<Server> servers) 
 ? servers
 : Enumerable.Empty<Server>();

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<string, List<Server>>(StringComparer.OrdinalIgnoreCase);

Now the keys is case-insensitive also during adding.

• paparazzo: ContainsKey is O(1) not O(n)
• t3chb0t: @Paparazzi I'm refering to the second example where OP uses _appServers.Where and not the ContainsKey.

Answer 2

If you are going to use Linq like that then you don't even need Dictionary. I am not getting the AddRange?

This would be so much less code with just add Application to Server and use a List with Linq. Server class does not know Application and I bet that would be useful.

With List LINQ

public class Server
{
 public string Name { get; set; }
 public string Family { get; set; }
 public string Environment { get; set; }
 public string Application { get; set; }
}
public class ServersSCV
{
 string fileName;
 List<Server> servers = new List<Server>();
 public IEnumerable<Server> Servers { get { return servers.OrderBy(x => x.Application).ThenBy(x => x.Family).ThenBy(x => x.Name); } }
 public IEnumerable<Server> GetServerByApplicastion(string Application)
 {
 return servers.Where(x => string.Compare(x.Application, Application, true) == 0).OrderBy(x => x.Family).ThenBy(x => x.Name);
 }
 public void RefreshData()
 {
 servers.Clear();
 using (TextReader textReader = File.OpenText(fileName))
 using (CsvReader reader = new CsvReader(textReader))
 {
 while (reader.Read())
 {
 Server server = reader.GetRecord<Server>();
 servers.Add(server);
 }
 }
 }
 public ServersSCV (string FileName)
 {
 fileName = FileName;
 RefreshData();
 }
}

• Tyler Daniels: I see what you mean (and it would be easier), but the problem I have with that approach is the application name isn't a property of the server. The .csv just uses it as a key for the association between them.
• paparazzo: @SeeSharpCode On the surface "software application" sure sounds like a property of the server. Just the fact you search on it I would call it a property of the server. Adding it as property breaks nothing. At any rate this lets you search on it with a simpler structure.