0
\$\begingroup\$

I am about to post a question to Stack Overflow about how to do a better job adding a number to the list inside the User class. But I feel if I refactored this I might solve my problem with coupling my data like this. So I am open to suggestions.

Ideone

public static void addOrUpdate(Dictionary<int, User> dic, int key, User user)
{
    //sets a new user data and just replaces it in the dictionary
    //var used = new User { ID = "id1", Name = "Harry" };
    var used = new User{ ID = "id3", Name = "Henry" ,AddressBook = new List<ContactNumber>(){new ContactNumber(3111),new ContactNumber(4444)}};

    if (dic.TryGetValue(key, out user))
    {
        // yay, value exists!
        dic[key] = used;

    }
    else
    {
        // darn, lets add the value 
        dic.Add(key, used);
    }
}
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Read the MSDN documentation, you'll see there is a ContainsKey method that will work much better than that TryGetValue mess. \$\endgroup\$ – radarbob Oct 21 '15 at 20:40
  • \$\begingroup\$ good suggestion Thank you, I was curious about that TryGetValue, something about that out in (key, out Value) seemed to be pointless. But hey it worked \$\endgroup\$ – h4mme7 Oct 21 '15 at 20:44
  • \$\begingroup\$ Could you explain why you are creating a used variable and not using the passed in user? As of now you are just going to replace the passed in variable with the variable held in the Dictionary if it exists. \$\endgroup\$ – Craig Russell Oct 21 '15 at 20:57
  • \$\begingroup\$ The idea was to identify the user based on the int Key in the Dictionary, or the name in the user class. and then update that users information. ( this is where I was having trouble). Would I copy the data to a new user was a thought and that is why i was passing the user to the update method. But understanding how to update just the list inside of user becomes a problem. \$\endgroup\$ – h4mme7 Oct 21 '15 at 21:03
7
\$\begingroup\$

You neither need TryGetValue nor ContainsKey if you don't care about the existence of the User in the Dictionary<TKey, TValue> which seems to be the case, because you want to either add or update the User in the dict.

You can simply use the Item property setter to achieve this and after changing the method name to AddOrUpdate (based on the NET naming guidelines) your former method will look like so

public static void AddOrUpdate(Dictionary<int, User> dic, int key, User user)
{
    //sets a new user data and just replaces it in the dictionary
    //var used = new User { ID = "id1", Name = "Harry" };
    var used = new User{ ID = "id3", Name = "Henry" ,AddressBook = new List<ContactNumber>(){new ContactNumber(3111),new ContactNumber(4444)}};

    dic[key] = used;

}  

This is because internally the Dictionary.Add() method calls Insert() with true for the add parameter like so

public void Add(TKey key, TValue value) {
    Insert(key, value, true);
}  

and the Item property looks like so

public TValue this[TKey key] {
    get {
        int i = FindEntry(key);
        if (i >= 0) return entries[i].value;
        ThrowHelper.ThrowKeyNotFoundException();
        return default(TValue);
    }
    set {
        Insert(key, value, false);
    }
}

where the add parameter of the Insert() method call is false.

\$\endgroup\$
6
  • \$\begingroup\$ Does it mean that it is somehow possible to disable this line ThrowHelper.ThrowKeyNotFoundException(); in the getter and use the return default(TValue); instead if key not found? I often happen to new the this in a derived dictionary so that it doesn't throw on non-existing key. \$\endgroup\$ – t3chb0t Oct 22 '15 at 5:16
  • \$\begingroup\$ @t3chb0t if you need to get a value based on a key you need to use TryGetValue() if you aren't sure if the key exists. \$\endgroup\$ – Heslacher Oct 22 '15 at 5:22
  • \$\begingroup\$ Sure thing ;-) but the default indexer is often more comfortable than the TryGetValue so I rather override the this and return a default value, usually when the value is a reference type. After seeing the source code in your answer I had hoped I can next time just disable the line that throws the exception ;-) \$\endgroup\$ – t3chb0t Oct 22 '15 at 5:28
  • \$\begingroup\$ @t3chb0t sure you can overide the indexer, but IMO thats not that good, because the expected behaviour of a dictionary is to throw if the key is not found. If you don't call the derived class Dictionary or something like this, you should be ok. \$\endgroup\$ – Heslacher Oct 22 '15 at 5:37
  • 2
    \$\begingroup\$ @t3chb0t - if you return default(TValue) for the getter you lose the information on whether the key was there and it's got the default value of the type or whether it simply wasn't there. In some cases that won't matter, but in the general case it is an important distinction. \$\endgroup\$ – RobH Oct 22 '15 at 8:00

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.