A better way of reading from a Dictionary



  • Here's a representative line:

    x = someDictionary.SingleOrDefault(x => x.Key.Equals(someKey));

    For those of you not familiar with C#, all this code does is retrieve a value from a hashmap.  It should look like this:

    x = someDictionary[someKey];

     

    By using SingleOrDefault, this actually will loop through every entry in the dictionary, searching for the key.




  • Actually, your version would throw if someKey can't be found.



    A correct new version would be:



    if (!someDictionary.TryGetValue(someKey, out x) x = default([i]type of x[/i])



    Unless of course someDictionary uses a non default IEqualityComparer...



  • Don't mess with it. If you speed it up, the code that depends on it being slow might start breaking. Just ask snoofle: http://forums.thedailywtf.com/forums/t/26436.aspx



  • @sobani said:

    A correct new version would be:



    if (!someDictionary.TryGetValue(someKey, out x) x = default(type of x)

    Which is then useful enough to factor out as an extension method.



  • @sobani said:

    Actually, your version would throw if someKey can't be found.

    A correct new version would be:

    if (!someDictionary.TryGetValue(someKey, out x) x = default(type of x)

    Unless of course someDictionary uses a non default IEqualityComparer...
     

    Wait, so TryGetValue doesn't automatically set the out param to Default if it's not found?



  • @ThingGuy McGuyThing said:

    By using SingleOrDefault, this actually will loop through every entry in the dictionary, searching for the key.
     

    Yes, but have you realized HOW MUCH cpu time did the code save by not having to check whether the key exists???



  • @sobani said:

    Actually, your version would throw if someKey can't be found.
     

    Trying to retrieve an item from a hashmap which doesn't exist throws an exception? And then you have to go through hackery with TryGetValue if you want to avoid it? Brillant!



  • @pbean said:

    @sobani said:

    Actually, your version would throw if someKey can't be found.
     

    Trying to retrieve an item from a hashmap which doesn't exist throws an exception? And then you have to go through hackery with TryGetValue if you want to avoid it? Brillant!

     Especially since a dictionary doesn't include a method like ContainsKey, amiright?


  • @rad131304 said:

    @pbean said:

    @sobani said:

    Actually, your version would throw if someKey can't be found.
     

    Trying to retrieve an item from a hashmap which doesn't exist throws an exception? And then you have to go through hackery with TryGetValue if you want to avoid it? Brillant!

     Especially since a dictionary doesn't include a method like ContainsKey, amiright?
     

    Sure, let's do two hash map lookups just to extract an item. That won't make it twice as slow at all.

     



  • @pbean said:

    @rad131304 said:

    @pbean said:

    @sobani said:

    Actually, your version would throw if someKey can't be found.
     

    Trying to retrieve an item from a hashmap which doesn't exist throws an exception? And then you have to go through hackery with TryGetValue if you want to avoid it? Brillant!

     Especially since a dictionary doesn't include a method like ContainsKey, amiright?
     

    Sure, let's do two hash map lookups just to extract an item. That won't make it twice as slow at all.

     

    If you cared about performance, you wouldn't use a hashtable.

    You'd obviously use a stored procedure in an Oracle database.



  • @Ben L. said:

    @pbean said:
    @rad131304 said:
    @pbean said:
    @sobani said:
    Actually, your version would throw if someKey can't be found.
    Trying to retrieve an item from a hashmap which doesn't exist throws an exception? And then you have to go through hackery with TryGetValue if you want to avoid it? Brillant!

     Especially since a dictionary doesn't include a method like ContainsKey, amiright?
    Sure, let's do two hash map lookups just to extract an item. That won't make it twice as slow at all.

     

    If you cared about performance, you wouldn't use a hashtable.

    You'd obviously use a stored procedure in an Oracle database.

    Which version of Oracle?

     



  • @sobani said:

    Actually, your version would throw if someKey can't be found.



    A correct new version would be:



    if (!someDictionary.TryGetValue(someKey, out x) x = default(type of x)



    Unless of course someDictionary uses a non default IEqualityComparer...

    I prefer this one-liner:

    SomeType value = (new SomeType[] {null, someDictionary[key] })[Convert.ToInt32(someDictionary.ContainsKey(key))];
    

    Disclaimer: no idea if this would actually compile, but anyways that would be QA's problem, not mine.



  • @Speakerphone Dude said:

    @sobani said:
    Actually, your version would throw if someKey can't be found.



    A correct new version would be:



    if (!someDictionary.TryGetValue(someKey, out x) x = default(type of x)



    Unless of course someDictionary uses a non default IEqualityComparer...

    I prefer this one-liner:

    SomeType value = (new SomeType[] {null, someDictionary[key] })[Convert.ToInt32(someDictionary.ContainsKey(key))];
    

    Disclaimer: no idea if this would actually compile, but anyways that would be QA's problem, not mine.

    That would still throw an exception if someDictionary[key] alone would throw an exception. Although it does get rid of the special case where someDictionary.ContainsKey(key) returns false and someDictionary[key] returns a Ford F-150.



  • @Ben L. said:

    @Speakerphone Dude said:
    @sobani said:
    Actually, your version would throw if someKey can't be found.



    A correct new version would be:



    if (!someDictionary.TryGetValue(someKey, out x) x = default(type of x)



    Unless of course someDictionary uses a non default IEqualityComparer...

    I prefer this one-liner:

    SomeType value = (new SomeType[ {null, someDictionary[key] })[Convert.ToInt32(someDictionary.ContainsKey(key))];
    

    Disclaimer: no idea if this would actually compile, but anyways that would be QA's problem, not mine.

    That would still throw an exception if someDictionary[key] alone would throw an exception. Although it does get rid of the special case where someDictionary.ContainsKey(key) returns false and someDictionary[key] returns a Ford F-150.

    Is it possible to embed Try-Catches in statements? That would be hugely convenient for one-liners.



  • @pbean said:

    Trying to retrieve an item from a hashmap which doesn't exist throws an exception?
    Not sure if serious...

    What would you suggest instead? Returning null? What if you want to store new values?



  • @Zecc said:

    @pbean said:
    Trying to retrieve an item from a hashmap which doesn't exist throws an exception?
    Not sure if serious...

    What would you suggest instead? Returning null? What if you want to store new values?

     

    Pretty sure I'm serious. Yes, I would indeed suggest returning null. I assume you mean "what if you want to store null values?"? Well, you shouldn't. Why would you? In Java, Map.get() returns null if there is no entry for the given key, and I haven't had any problems with that all the while.

    Also, it makes the dictionary[] notation rather useless, because most of the times you're working with maps you wouldn't really want to be catching exceptions (or rather you don't want to have them thrown), it's just a waste of cpu cycles. And if you are certain the key has a value (so you won't get the exception), you probably don't need the map any way (unless you first call ContainsKey, which in turn is inefficient). So then the only option is to use the TryGetValue(), they might as well scrap the []/get from dictionary.

    (If you meant "store new values", I'm not sure I understand what you're saying).

     



  • @Mason Wheeler said:

    Wait, so TryGetValue doesn't automatically set the out param to Default if it's not found?

    It will, but I consider that an implementation detail.

    @pbean said:

    Pretty sure I'm serious. Yes, I would indeed suggest returning null. I assume you mean "what if you want to store null values?"? Well, you shouldn't. Why would you? In Java, Map.get() returns null if there is no entry for the given key, and I haven't had any problems with that all the while.

    And what does Java return if TValue is int, 0? I'd rather know the difference between a value set to 0 and a value not set at all. Besides -1 is a much better default value anyway.

    if (dict.TryGetValue(key, out value)) /* do something with value */
    else /* do something without value */

    is superior to

    value = dict[key]
    if (value != null) /* do something with value */
    else /* do something without value */



  • @pbean said:

    @rad131304 said:

    @pbean said:

    @sobani said:

    Actually, your version would throw if someKey can't be found.
     

    Trying to retrieve an item from a hashmap which doesn't exist throws an exception? And then you have to go through hackery with TryGetValue if you want to avoid it? Brillant!

     Especially since a dictionary doesn't include a method like ContainsKey, amiright?
     

    Sure, let's do two hash map lookups just to extract an item. That won't make it twice as slow at all.

     

     

     

    Seeing some of these postings, I begin to understand why concurrency and multi-threaded programming leads to some of the WTFs that show up here. (not pointing at pbean, who noted that the double lookup was sub-optimal, although he, like many others, did not mention race conditions)

     



  • @sobani said:

    @pbean said:
    Pretty sure I'm serious. Yes, I would indeed suggest returning null. I assume you mean "what if you want to store null values?"? Well, you shouldn't. Why would you? In Java, Map.get() returns null if there is no entry for the given key, and I haven't had any problems with that all the while.
    And what does Java return if TValue is int, 0?
    Since int is a primitive type, and primitives can not be type parameters for Generics in java, TValue can not be int. You can use the non-primitive Integer, but since that is, basically, an ordinary object type, it is nullable, so you would still get a null result.

     


  • :belt_onion:

    @pbean said:

    @Zecc said:

    @pbean said:
    Trying to retrieve an item from a hashmap which doesn't exist throws an exception?
    Not sure if serious...

    What would you suggest instead? Returning null? What if you want to store new values?

     

    Pretty sure I'm serious. Yes, I would indeed suggest returning null. I assume you mean "what if you want to store null values?"? Well, you shouldn't. Why would you? In Java, Map.get() returns null if there is no entry for the given key, and I haven't had any problems with that all the while.

    Also, it makes the dictionary[ notation rather useless, because most of the times you're working with maps you wouldn't really want to be catching exceptions (or rather you don't want to have them thrown), it's just a waste of cpu cycles. And if you are certain the key has a value (so you won't get the exception), you probably don't need the map any way (unless you first call ContainsKey, which in turn is inefficient). So then the only option is to use the TryGetValue(), they might as well scrap the [/get from dictionary.

    (If you meant "store new values", I'm not sure I understand what you're saying).

    Seriously, are you pissed off at the fact that .NET offers the developers a choice on how retrieving a value from a dictionary works? You're probably just jealous that Java forces you to use the one and only way :)

    I have used both methods in the past:

    • Either the value you are looking for MUST be present in the dictionary. If it's not there,it's an exception and the processing cannot continue --> In this case, I want the dictionary to throw the exception and it can
    • Either I'm just checking if something is present. If it's present I treat it; if it's not present I don't treat it so I want my dictionary to behave as you describe and it can.

     



  • @pbean said:

    @rad131304 said:

    @pbean said:

    @sobani said:

    Actually, your version would throw if someKey can't be found.
     

    Trying to retrieve an item from a hashmap which doesn't exist throws an exception? And then you have to go through hackery with TryGetValue if you want to avoid it? Brillant!

     Especially since a dictionary doesn't include a method like ContainsKey, amiright?
     

    Sure, let's do two hash map lookups just to extract an item. That won't make it twice as slow at all.

    Fair enough. I don't work with hashmaps big enough where two key lookups is a significant performance hit, so I wouldn't have even thought to look at it as a bottleneck.



  • @sobani said:

    @Mason Wheeler said:

    Wait, so TryGetValue doesn't automatically set the out param to Default if it's not found?

    It will, but I consider that an implementation detail.

    A method with an out parameter has to set that parameter to something, so TryGetValue has to set it to default(T) in any implementation.



  • @pbean said:

    And if you are certain the key has a value (so you won't get the exception), you probably don't need the map any way
     

    wha?



  • God fucking jesus! I knew there is a reason i hate most "classic" languages.

    In pythong you can just do

    mydict[key] # throws exception if key isn't found

     or

    mydict.get(key,default)  # returns default if key isn't found

    How hard is that? And why C#/Java/Whatever has to go through a string of WTF-ery to provide the same thing?



  • I don't see how your pythong (seriously?) example differs from the C# one.


  • ♿ (Parody)

    @blakeyrat said:

    I don't see how your pythong (seriously?) example differs from the C# one.

    Yeah, that's the problem with a language that combines syntactically significant whitespace and sexy underwear.



  • Yeah, I'm going to try the veal.



  • @bjolling said:

    Seriously, are you pissed off at the fact that .NET offers the developers a choice on how retrieving a value from a dictionary works? You're probably just jealous that Java forces you to use the one and only way :)

    I have used both methods in the past:

    • Either the value you are looking for MUST be present in the dictionary. If it's not there,it's an exception and the processing cannot continue --> In this case, I want the dictionary to throw the exception and it can
    • Either I'm just checking if something is present. If it's present I treat it; if it's not present I don't treat it so I want my dictionary to behave as you describe and it can.

     

     

    I'm not really pissed off (not sure how my post could come across as pissed off), I just don't see the use at all. To be honest, I have never experienced the first use case, at least as far as I can remember. It's mostly just a bug in the code, like an array index out of bounds exception: it exists to prevent you from doing something silly (accessing an array index which does not exist), and in nearly all cases it just points to a bug somewhere in your code.

    Also I don't find this out syntax very appealing. It just personal preference, really. I don't mind doing a null check instead of a TryGetValue. In the end you could just write them the same way:

    [code]var something;
    if (dict.TryGetValue(key, out someting)) {
    /* do something useful */
    } else {
    /* do something else */
    } [/code]

    [code]String something;
    if ((something = map.get(key)) != null) {
    /* do something useful */
    } else {
    /* do something else */
    } [/code]

     Only difference is that TryGetValue sets something to the default if it fails.

    @jes said:

    Seeing some of these postings, I begin to understand why concurrency and multi-threaded programming leads to some of the WTFs that show up here. (not pointing at pbean, who noted that the double lookup was sub-optimal, although he, like many others, did not mention race conditions)
     

    To be fair I'm still only a junior developer and haven't been involved much with multi-threading. But that's why we have code reviews, and the seniors will point out any race conditions for me, allowing me to learn (which has happened before, so I trust in the process). In fact, I want to thank you for bringing this to my attention. :)

     

     



  • @pbean said:

    To be honest, I have never experienced the first use case, at least as far as I can remember. It's mostly just a bug in the code, like an array index out of bounds exception: it exists to prevent you from doing something silly (accessing an array index which does not exist), and in nearly all cases it just points to a bug somewhere in your code.
    There you go. Wouldn't you say it's better to have an exception thrown when accessing the dictionary with a wrong key rather than having a null reference exception further down the line?

    And yes, I did mean null value rather than new value back in the other post.



  • @blakeyrat said:

    I don't see how your pythong (seriously?) example differs from the C# one.
    The difference is that in Python you can specify the default value in the get() call. It doesn't necessarily use the default value for the type. Which means you can write:

    # cfg is a dictionary
    flavour = cfg .get('flavour', 'vanilla')
    topping = cfg.get('topping', 'chocolate cookies')

    The default default is None, which is Python's null.



  • Well that's great but we were talking about pythong.



  • @blakeyrat said:

    Well that's great but we were talking about pythong.
    Where's the guy who complains about lame jokes when you need him?



  •  He's right there.



  • How'd he get into my house?



  • @Zecc said:

    @pbean said:

    To be honest, I have never experienced the first use case, at least as far as I can remember. It's mostly just a bug in the code, like an array index out of bounds exception: it exists to prevent you from doing something silly (accessing an array index which does not exist), and in nearly all cases it just points to a bug somewhere in your code.
    There you go. Wouldn't you say it's better to have an exception thrown when accessing the dictionary with a wrong key rather than having a null reference exception further down the line?

     

    An array index which is out of bounds is inaccessible, you can't put data there, and it cannot have any data. However, any array index which is unfilled (ie. it's within bounds but not object has been assigned there) returns null (or possibly the default value of a primitive type, if the array contains values of a primitive type). There difference is that a dictionary / map does not (normally) have any bounds, so any element you request will not be out of bounds. Instead, similar to the array, you will try access an element at a place (index) which was not filled with a value before. This does not throw an exception for arrays, and it should not for dictionaries. And a common pattern is to try and retrieve a value from a map, if it was not in there, create a new object and put it in there, then continue with either the retrieved or new object. So requesting the element with that key is not an exceptional situation, it's a common situation. It should not throw an exception.

     


  • :belt_onion:

    @pbean said:

    @bjolling said:

    Seriously, are you pissed off at the fact that .NET offers the developers a choice on how retrieving a value from a dictionary works? You're probably just jealous that Java forces you to use the one and only way :)

    I have used both methods in the past:

    • Either the value you are looking for MUST be present in the dictionary. If it's not there,it's an exception and the processing cannot continue --> In this case, I want the dictionary to throw the exception and it can
    • Either I'm just checking if something is present. If it's present I treat it; if it's not present I don't treat it so I want my dictionary to behave as you describe and it can.

     

     

    I'm not really pissed off (not sure how my post could come across as pissed off), I just don't see the use at all. To be honest, I have never experienced the first use case, at least as far as I can remember. It's mostly just a bug in the code, like an array index out of bounds exception: it exists to prevent you from doing something silly (accessing an array index which does not exist), and in nearly all cases it just points to a bug somewhere in your code.

    Example of first use case: for a project I worked on, user preferences where loaded from a key/value table and stored in a dictionary. So if an options like userOptions[language] is missing from the table, it means the userprofile was incomplete in the database. I prefer to get an exception when trying to get the value because it will be something like: "Cannot find key language" because it's informational. I don't like getting the exception later on in the processing: "unable to set the Thread.CurrentUICulture to 'null'" because it's further away from the root cause.



  • When I want to read from a config, I use a config object. When I want to read from a hashtable, I use a map/dictionary. The config object may be backed by a map, but only it would be responsible for deciding the appropriate action for missing data and defaults. A fundamental data structure should not. You wouldn't expect a string to throw an exception if it couldn't find indexOf(), would you?

    This is my opinion. So it is written, so shall it be.



  • @Xyro said:

    When I want to read from a config, I use a config object. When I want to read from a hashtable, I use a map/dictionary. The config object may be backed by a map, but only it would be responsible for deciding the appropriate action for missing data and defaults. A fundamental data structure should not. You wouldn't expect a string to throw an exception if it couldn't find indexOf(), would you?

    This is my opinion. So it is written, so shall it be.

    In Go, map[key]value is part of the language. That's right, the language, not the library. To get something from it, you use the sensible value := mapname[key]. If you don't want it to panic if there's no such key, you can do value, exists := mapname[key] and value will be set to the zero value for its type and exists will be false if the key is not in the map.


  • The sensible use of multiple return values is definitely a nice feature of Go. Hard to not get skeeved out over its syntax, though. Declaration/assignment shorthand, function import shorthand, constant shorthand, what's with all that? I mean, its pointer syntax is way better than C, though I still attest that the most confusing thing about pointers is not the concept but the syntax. Ah well, the more I stare at it, the less weird it seems.

    If only it could get rid of that new vs make nonsense. Oh, that and the array/slice business. Wtf is that.

    Anyway, you seem to have been working with it for a while. How do you like it?



  • @bjolling said:

    Example of first use case: for a project I worked on, user preferences where loaded from a key/value table and stored in a dictionary. So if an options like userOptions[language] is missing from the table, it means the userprofile was incomplete in the database. I prefer to get an exception when trying to get the value because it will be something like: "Cannot find key language" because it's informational. I don't like getting the exception later on in the processing: "unable to set the Thread.CurrentUICulture to 'null'" because it's further away from the root cause.
     

    Better yet, check while loading the data, rather than when trying to get. Fail early, so the users do not go through half a process before discovered they are unable to continue due to some missing user options.

     



  • @Xyro said:


    If only it could get rid of that new vs make nonsense. Oh, that and the array/slice business. Wtf is that.

    new creates a pointer to the zero value for a type. maps and slices have a zero value of nil, so that would be pretty damn useless in that case. Arrays are like C arrays, slices can be resized.

  • :belt_onion:

    @pbean said:

    @bjolling said:

    Example of first use case: for a project I worked on, user preferences where loaded from a key/value table and stored in a dictionary. So if an options like userOptions[language] is missing from the table, it means the userprofile was incomplete in the database. I prefer to get an exception when trying to get the value because it will be something like: "Cannot find key language" because it's informational. I don't like getting the exception later on in the processing: "unable to set the Thread.CurrentUICulture to 'null'" because it's further away from the root cause.
     

    Better yet, check while loading the data, rather than when trying to get. Fail early, so the users do not go through half a process before discovered they are unable to continue due to some missing user options.

     

    But not too early either. I actually prefer to only check at the start of a use case in which I need the value. Why bother annoying the user with "Cannot find 'this setting' in your profile" if he is nowhere near a user case that requires that setting. For example, only at the start of "Ship my purchase" use-case, I will check if there is a shipping address. I won't check that when the user is browsing the catalog

    ...Although I must admit that 'shipping address' is not a good choice for a example of a user setting but I can't come up with something else...erm...maybe 'favourite theme' on a page that cannot be customized..i dunno :)



  • @Ben L. said:

    new creates a pointer to the zero value for a type. maps and slices have a zero value of nil, so that would be pretty damn useless in that case.

    So why not just use a normal constructor like all the other objects? Why are certain datatypes blessed by the language and others not?

    @Ben L. said:

    Arrays are like C arrays, slices can be resized.

    So slices are like vectors, except you have to handle everything manually..? I understand why it would be very useful to distinguish between, for example, requiring a parameter of a certain-sized-array-like-thing and an arbitrary-sized-array-like-thing, but I don't see the benefit of a slice, which feels like a half-breed between a vector and an array.

    Explain please! Meanwhile, I'll read over Effective Go again, because I'm sure I missed things the last time I looked at the language.


Log in to reply