Yes a `map-entry?` predicate was sort of the direction I was leaning in that would help avoid issues like this around these changes.
On Friday, October 16, 2015 at 10:58:47 AM UTC-5, Alex Miller wrote: > > Perhaps having a "map-entry?" predicate that was smarter would be helpful. > > On Friday, October 16, 2015 at 9:39:09 AM UTC-5, Mike Rodriguez wrote: >> >> Yes, I am in support of the fact that size=2 vectors now can now have >> `key` and `val` called on them. This not working prior to Clojure 1.8 was >> occasionally the reason why I just used `first` and `second` instead of >> `key` and `val` before since some function transformations could result in >> size=2 vectors rather than true map entries. >> >> I think that supporting `empty` on MapEntry is weird, as Alex said above. >> It does sound the same as the record case. >> >> The only frustration I have with this change is that not all >> APersistentVectors really act as a map entry. Only those of size=2 do. So >> the interface becomes misleading if you try to use it to figure out from a >> generic standpoint, what things are map entries and what things aren't. >> The ambiguity is that all vectors look like map entries. I guess you >> could do the >> (instance? IMapEntry x) check along with (== 2 (count x)) for the vector >> case... >> >> On Friday, October 16, 2015 at 6:39:50 AM UTC-5, Alex Miller wrote: >>> >>> >>> On Friday, October 16, 2015 at 3:32:43 AM UTC-5, Pierre-Yves Ritschard >>> wrote: >>>> >>>> Hi Mike, >>>> >>>> The code at here seems to contradict you: >>>> >>>> https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/APersistentVector.java#L448-L460, >>>> >>>> >>>> as does "(key [:a :b])" in the REPL. >>>> >>>> The only limitation is that vectors need to be of size two to act as >>>> IMapEntries (otherwise an IllegalOperation exception is thrown). >>>> >>>> The change seems logical and allows key & val to be used more >>>> generically. >>>> >>>> You're right that will fail on code that checks for (instance? >>>> IMapEntry). >>>> >>> >>> APersistentVector implements IMapEntry, so this doesn't seem correct. >>> >>> >>>> A good alternative - paging Alex Miller - could be for (empty) on a >>>> MapEntry to return an empty PersistentVector instead of nil, which >>>> would >>>> ensure that calls to (into (empty <map-entry>) (map f <map-entry>)) >>>> would return a valid map entry (instead of a collection). >>>> >>> >>> It does not make sense to me for empty on a MapEntry to do what you ask >>> (for similar reasons why empty on a record is not allowed). >>> >>> >>>> I'm happy to create a ticket for this use-case if deemed valid. >>>> >>>> Cheers, >>>> - pyr >>>> >>>> >>>> >>>> On 10/16/2015 01:28 AM, Mike Rodriguez wrote: >>>> > Someone else looked at the issue on >>>> https://github.com/ztellman/riddley/issues/18 >>>> > >>>> > This issue makes the current version of riddley, and therefore >>>> potemkin, not work on Clojure 1.8 beta1 >>>> > >>>> > There is a pull request to fix it at >>>> https://github.com/ztellman/riddley/pull/19 >>>> > >>>> > However I am wondering if it is going to affect more places. The >>>> problem is that in Clojuee 1.8 APersistentVector now implements IMapEntry >>>> (therefore j.u.Map$Entry as well), but it doesn't implement the key or val >>>> methods. >>>> > What is the reason for that change and/or is this a desired side >>>> effect of the change? >>>> > >>>> >>> -- You received this message because you are subscribed to the Google Groups "Clojure" group. To post to this group, send email to clojure@googlegroups.com Note that posts from new members are moderated - please be patient with your first post. To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en --- You received this message because you are subscribed to the Google Groups "Clojure" group. To unsubscribe from this group and stop receiving emails from it, send an email to clojure+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
