5
\$\begingroup\$

Since I couldn't find any good answers I made my own class called DuplicateDictionary for personal usage. I would like some tips to improve it.

public class DuplicateDictionary<TKey, TValue>: List<KeyValuePair<TKey, TValue>>
{
    public DuplicateDictionary()
    {

    }

    public DuplicateDictionary(List<KeyValuePair<TKey, TValue>> list)
    {
        foreach(KeyValuePair<TKey, TValue> kvp in list)
        {
            this.Add(kvp);
        }
    }

    public DuplicateDictionary(Dictionary<TKey, TValue> dictionary)
    {
        foreach(KeyValuePair<TKey, TValue> kvp in dictionary)
        {
            this.Add(kvp);
        }
    }

    public TValue this[TKey index]
    {
        get
        {
            this.ContainsKey(index);
            return this[index];
        }
        set
        {
            this[index] = value;
        }
    }

    public void Add(TKey key, TValue value)
    {
        this.Add(new KeyValuePair<TKey, TValue>(key, value));
    }

    public bool ContainsKey(TKey key)
    {
        foreach(KeyValuePair<TKey, TValue> kvp in this)
            if(kvp.Key.Equals(key)) return true;

        return false;
    }
}

This class is useful for log things. I use it to log a calculator's past functions and numbers, and once equals is pressed, adds all the past numbers with the functions.

\$\endgroup\$
  • 8
    \$\begingroup\$ I think you should provide an example on how some1 is supposed to use this class. How are you going to access duplicate values? If you can't then why store them? Also your indexer looks really fishy. \$\endgroup\$ – Nikita B Aug 27 '15 at 7:02
  • \$\begingroup\$ Refer to the edit \$\endgroup\$ – hexagonest Aug 28 '15 at 3:49
15
\$\begingroup\$

Such a data structure already exists, and is called ILookup<TKey, TElement>. It can be created using the ToLookup extension.

The ToLookup<TSource, TKey>(IEnumerable<TSource>, Func<TSource, TKey>) method returns a Lookup<TKey, TElement>, a one-to-many dictionary that maps keys to collections of values. A Lookup<TKey, TElement> differs from a Dictionary<TKey, TValue>, which performs a one-to-one mapping of keys to single values.

var list = new List<Tuple<string, int>>
{
    Tuple.Create("a", 1),
    Tuple.Create("a", 2),
    Tuple.Create("a", 3),
    Tuple.Create("b", 4),
    Tuple.Create("c", 5)
};

var lookup = list.ToLookup(t => t.Item1, t => t.Item2);

foreach(var kv in lookup)
{
    Console.Write(kv.Key);
    Console.WriteLine(" - " + string.Join(", ", kv));
}

// prints
// a - 1, 2, 3
// b - 4
// c - 5

As Vince pointed out, your DuplicateDictionary will suffer from slow access. Lookup won't.

\$\endgroup\$
11
\$\begingroup\$

First I second what Vince Panuccio stated in his answer

I don't think this class should have a reason to exist. A key is just that, a key. If you have duplicate keys and duplicate values what you're essentially after is a grouping or a dictionary or with a set or list as its value.


Bug alert

This will break with an StackOverflowException

public TValue this[TKey index]
{
    get
    {
        this.ContainsKey(index);
        return this[index];
    }
    set
    {
        this[index] = value;
    }
}  

by calling this.ContainsKey() which again refers to the Item (this[TKey]) property and if we would omit this call, it would just break again with a StackOverflowException based on the return this[index]; which is reffering the property getter itself again.

Setting aside this big bug, an argument named index usually indicates some kind of numerical type. So a better parameter name would be in this case just key.


The ctor

public DuplicateDictionary(List<KeyValuePair<TKey, TValue>> list)
{
    foreach(KeyValuePair<TKey, TValue> kvp in list)
    {
        this.Add(kvp);
    }
}  

could be easily improved by using the AddRange() method of the List<T> like so

public DuplicateDictionary(List<KeyValuePair<TKey, TValue>> list)
{
    this.AddRange(list);
}

but that isn't really needed in this way because you also have a ctor which takes an Dictionary<TKey, TValue> as a parameter. I suggest to replace both ctors by a different one which only takes an IEnumerable<KeyValuePair<TKey, TValue>> like so

public DuplicateDictionary(IEnumerable<KeyValuePair<TKey, TValue>> items)
{
    this.AddRange(items);
}  

which can be called using a Dictionary and a List.

This looks good but we can still do better by using the ctor of the List instead like so

public DuplicateDictionary(IEnumerable<KeyValuePair<TKey, TValue>> items)
    : base(items)
{  } 
\$\endgroup\$
  • \$\begingroup\$ What does base do? Excuse my ignorance \$\endgroup\$ – hexagonest Aug 27 '15 at 9:11
  • \$\begingroup\$ Thats the calling of the ctor of the inherited class \$\endgroup\$ – Heslacher Aug 27 '15 at 9:12
7
\$\begingroup\$

It might help if you explain the problem you are trying to solve first, but I'll have a crack at this.

I don't think this class should have a reason to exist. A key is just that, a key. If you have duplicate keys and duplicate values what you're essentially after is a grouping or a dictionary with a set or list as its value.

What you have is a list of KeyValuePairs which is not the same as a Dictionary :-)

Dictionaries benefit from having unique keys by giving you fast lookups, I'm not sure what benefit you gain by having a DuplicateDictionary.

\$\endgroup\$
1
\$\begingroup\$

I think you want to have some kind of a multi value single key dictionary. So I suggest to use for example a List<> to hold the values.

Here's an example:

public class DuplicateDictionary<TKey, TValue> : Dictionary<TKey, List<TValue>>, IEnumerable<KeyValuePair<TKey, TValue>>
{
    public new IEnumerable<KeyValuePair<TKey, TValue>> this[TKey key]
    {
        get
        {
            List<TValue> values;
            if (!TryGetValue(key, out values))
            {
                return Enumerable.Empty<KeyValuePair<TKey, TValue>>();
            }

            return values.Select(v => new KeyValuePair<TKey, TValue>(key, v));
        }
        set
        {
            foreach (var _value in value.Select(kvp => kvp.Value))
            {
                Add(key, _value);
            }
        }
    }



    public void Add(TKey key, TValue value)
    {
        List<TValue> values;
        if (!TryGetValue(key, out values))
        {
            values = new List<TValue>();
            Add(key, values);
        }
        values.Add(value);
    }

    public IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator()
    {
        foreach (var item in ((Dictionary<TKey, List<TValue>>)this))
        {
            foreach (var value in item.Value)
            {
                yield return new KeyValuePair<TKey, TValue>(item.Key, value);
            }
        }
    }
}

static void Main(string[] args)
{
    var dupDic = new DuplicateDictionary<string, string>();

    dupDic.Add("abc", "123");
    dupDic.Add("abc", "456");
    dupDic.Add("xyz", "789");
    dupDic.Add("xyz", "098");
    dupDic.Add("xyz", "290");

    foreach (var kvp in dupDic)
    {
        Console.WriteLine("Key = \"{0}\" Value = \"{1}\"", kvp.Key, kvp.Value);
    }

    Console.ReadKey();
}
\$\endgroup\$
0
\$\begingroup\$

You can List of KeyValuePair to store duplicates

Example :

List<KeyValuePair<string, string>> listKeyValPair= new List<KeyValuePair<string, string>>();

KeyValuePair<string, string> keyValue= new KeyValuePair<string, string>("KEY1", "VALUE1");

listKeyValPair.Add(keyValue);
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! Please read How do I write a good answer?: "Every answer must make at least one insightful observation about the code in the question." Please explain why your suggestion would be an improvement. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Feb 20 at 7:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.