Skip to content

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