Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread safe Dict doesnt return dict #18

Open
CumpsD opened this issue Mar 29, 2015 · 2 comments
Open

Thread safe Dict doesnt return dict #18

CumpsD opened this issue Mar 29, 2015 · 2 comments

Comments

@CumpsD
Copy link

CumpsD commented Mar 29, 2015

Any reason these operations dont return dict, while the non-Safe do?

https://github.com/jack-pappas/ExtCore/blob/master/ExtCore/Collections.Dict.fs#L331-L337

Why not:

let updateOrAdd key value (dict : dict<'Key, 'T>) =
    // Preconditions
    checkNonNull "dict" dict

    if dict.IsReadOnly then
        invalidOp "Cannot update or add an entry to a read-only dictionary."
    lock dict <| fun () ->
        dict.[key] <- value

    dict
@jack-pappas
Copy link
Owner

Hmm. There might have been a good reason for it at one point, but I don't remember now.

If I had to guess, it may be because returning the dictionary instance makes the function signatures more like those in the Map module.

Whatever the reason was before, the inconsistency seems like a code smell now so I'll keep this open as a work item for the first major ExtCore release (v1.0). There's no ETA for that release but I do have a few other small, breaking changes to make to other parts of the library at some point so it'd be best to group them all together.

@CumpsD
Copy link
Author

CumpsD commented Apr 18, 2015

Thanks :) I've temporarily included a modified version in my project ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants