nickbabcock / Pdoxcl2Sharp

A Paradox Interactive general file parser
MIT License
40 stars 13 forks source link

Replace Dictionary with an Insertion Perserving Collection #19

Open nickbabcock opened 10 years ago

nickbabcock commented 10 years ago

ReadDictionary returns a Dictionary, as one might expect. There is only one problem with this function and it is inherently tied to the return type. A Dictionary doesn't preserve insertion order. Ordinarily this isn't a problem, but what if the client of this library wants to write a dictionary in the same order that it was read. This is impossible.

So many times I've run into data like

flags={
    flag_a=1402.3.21
    flag_b=1403.2.1
    [...]
}

It feels wrong to create a class or action to define how to parse this when the functionality is already in the parser.

I'm unsure what the solution is. Ideally, the IDictionary<TKey, TValue> return type should be kept, but I'm not sure if that will be possible. My only idea involves creating a custom type with a dictionary and a list. The only time the list would come into play would be when GetEnumerator was called (so, whenever the client calls foreach on the collection). This kinda seems like a lot of overhead for something simple.

The alternative solution is to change the interface from IDictionary<TKey, TValue> to IList<KeyValuePair<TKey, TValue>> and make the client responsible for dictionary creation if they need it. If this route is taken, I would prefer the method to be renamed to something like ReadKeyValues

Measter commented 10 years ago

Why not use an OrderedDictionary?

nickbabcock commented 10 years ago

If only it was generic, it would be perfect.

TommasoBelluzzo commented 8 years ago

http://www.codeproject.com/Articles/18615/OrderedDictionary-T-A-generic-implementation-of-IO

nickbabcock commented 8 years ago

Thanks for the link! I think I would have to dig into it a lot more to see what the performance trade offs would be and if they'd be worth it. I've also had success using Tuple<string, 'T>[], so I'm not sure it's critical for this issue to be implemented.