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

feat: support clojure.core/get arity 3 #87

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: support clojure.core/get arity 3 #87

wants to merge 1 commit into from

Conversation

phrohdoh
Copy link
Contributor

https://clojure.github.io/clojure/clojure.core-api.html#clojure.core/get

Returns the value mapped to key, not-found or nil if key not present.


(get map key)           ;; supported prior to this PR
(get map key not-found) ;; supported as of this PR

Instead of adding a not_found: Option<&Rc<Value> to IPersistentMap::get (to be threaded through all nested calls and a seemingly-arbitrary default to ::get callsites) I opted to continue the *_with_* convention; creating a get_with_default function.


If merged, this would fix #86 (for maps, but I, perhaps incorrectly, recall reading that get can be applied to non-maps, though the linked docs certainly do not suggest so).

https://clojure.github.io/clojure/clojure.core-api.html#clojure.core/get
> Returns the value mapped to key, not-found or nil if key not present.

    (get map key)           ;; supported prior to this commit
    (get map key not-found) ;; supported as of this commit
let map = persistent_list_map!(map_entry!("x", "v"));
let key = Keyword::intern("k").to_rc_value();
let default = Keyword::intern("not-found").to_rc_value();
let val: Rc<Value> = map.get_with_default(&key, &default);
Copy link
Contributor Author

@phrohdoh phrohdoh Jul 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not intend to leave these type annotations in.

  • remove type annotations in get_with_default and get_with_default_empty tests

@phrohdoh
Copy link
Contributor Author

See phrohdoh/cljrs@ceb3f9e for changes building on top of this PR to support map and keyword application default values:

({:k :v} :x)            ;; => nil
({:k :v} :x :not-found) ;; => :not-found
(:x {:k :v})            ;; => nil
(:x {:k :v} :not-found) ;; => :not-found

pmap.get_with_default(key, not_found)
} else {
pmap.get(key)
}.to_value();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last part I'd like to add then is a test for GetFn (including one covering the original 2 arg'd GetFn we never wrote)

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

Successfully merging this pull request may close these issues.

clojure.core/get ought to support an optional 3rd arg: not-found (default value)
2 participants