3 ms·

That’s a very, very common source of subtle bugs (often exploitable as security bugs) that are hard to spot and hard to test for. It’s good practice to design

That’s a very, very common source of subtle bugs (often exploitable as security bugs) that are hard to spot and hard to test for. It’s good practice to design APIs to make race conditions less likely, by explicitly not splitting operations across multiple calls. Separating “exists” and “get” into separate calls is a disaster.

seanmcdirmid 7y ago
I would compare this to a map API: an access could throw if a key wasn't present, but you can check to see if the key was in there first anyways. As long as the map isn't being used concurrently, it isn't that much of a problem, and if it is being used concurrently, then you need a different design. In the former case, separating "exists" and "get" would be considered over-engineering. I guess databases and file systems are different because concurrent access is the norm, not the exception, and APIs should be designed accordingly. But for most of your in-memory structures, you probably don't want them dealing with concurrency because they aren't going to be able do that very well anyways (instead, deal with concurrency outside of the data structure and have them throw when inconsistencies from bad concurrency policies arise).
- XMPPwocky 7y ago
  For most implementations of maps (hashtables and various tree-ish things), even in the single-threaded case, doing .exists and then .get is about twice as expensive as doing a .get that returns an Option (or nullable reference, default value, whatever)- you have to do the map lookup twice. (Okay, probably not twice as slow, since the cache will be hot, but still.)
  - seanmcdirmid 7y ago
    Not twice as slow. But you are forgetting the case where you know the element is already in the map, and you want to call map.get() without wrapping it in an option.
    - pjc50 7y ago
      C# does this fairly nicely with an "out" parameter: https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2.trygetvalue?view=netframework-4.8 https://docs.microsoft.com/en-us/dotnet/api/system.collectio... Or if you "know" it's there call the version that throws an exception if it isn't.
      - seanmcdirmid 7y ago
        I haven't used that aspect of the Dictionary API at all in the 9 years programming C#, it is just too unwieldy to have to define a separate local variable to hold the result of the get. It also doesn't scope the out assignment (if the get fails, you can still read the out variable bound). I'm rarely in the case where I don't expect something to be in a map anyways, and when I do, I have an extension method with a default value returned if the key doesn't exist (or in some cases populate). Keeping expressions clean is important.

seanmcdirmid 7y ago

I would compare this to a map API: an access could throw if a key wasn't present, but you can check to see if the key was in there first anyways. As long as the map isn't being used concurrently, it isn't that much of a problem, and if it is being used concurrently, then you need a different design. In the former case, separating "exists" and "get" would be considered over-engineering. I guess databases and file systems are different because concurrent access is the norm, not the exception, and APIs should be designed accordingly. But for most of your in-memory structures, you probably don't want them dealing with concurrency because they aren't going to be able do that very well anyways (instead, deal with concurrency outside of the data structure and have them throw when inconsistencies from bad concurrency policies arise).

XMPPwocky 7y ago
For most implementations of maps (hashtables and various tree-ish things), even in the single-threaded case, doing .exists and then .get is about twice as expensive as doing a .get that returns an Option (or nullable reference, default value, whatever)- you have to do the map lookup twice. (Okay, probably not twice as slow, since the cache will be hot, but still.)
- seanmcdirmid 7y ago
  Not twice as slow. But you are forgetting the case where you know the element is already in the map, and you want to call map.get() without wrapping it in an option.
  - pjc50 7y ago
    C# does this fairly nicely with an "out" parameter: https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2.trygetvalue?view=netframework-4.8 https://docs.microsoft.com/en-us/dotnet/api/system.collectio... Or if you "know" it's there call the version that throws an exception if it isn't.
    - seanmcdirmid 7y ago
      I haven't used that aspect of the Dictionary API at all in the 9 years programming C#, it is just too unwieldy to have to define a separate local variable to hold the result of the get. It also doesn't scope the out assignment (if the get fails, you can still read the out variable bound). I'm rarely in the case where I don't expect something to be in a map anyways, and when I do, I have an extension method with a default value returned if the key doesn't exist (or in some cases populate). Keeping expressions clean is important.