ClojureScript

Add MapEntry type

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code and Test

Description

It would be nice to have a MapEntry type in ClojureScript.

Since the introduction of the map-entry? predicate in Clojure 1.8 it has been trivial to determine if an object represents an entry in a map. Being able to distinguish a map entry from other data types is really handy when walking data amongst other things. Currently, ClojureScript uses PersistentVectors to represent entries from PersistentArrayMaps and PersistentHashMaps. PersistentTreeMaps use their own nodes to represent entries and as such would be unaffected.

Besides making it possible to use the the map-entry? predicate across both CLJ and CLJS code, there may space/performance benefits to having a specialised type.

  1. CLJS-2013.1.patch
    28/Apr/17 3:58 PM
    14 kB
    Thomas Mulvaney
  2. CLJS-2013.patch
    26/Apr/17 11:05 AM
    14 kB
    Thomas Mulvaney

Activity

Hide
Thomas Mulvaney added a comment -

I've attached a patch which introduces the MapEntry type. It's there but not hooked up - PAMs and PHMs don't emit them when you `seq` or `iterate` over them yet.

There is a set of tests which check the protocols all map entries should implement: IVector, IAssociative, ISeq etc. The exact same set of tests are run across PersistentVector, RedNode, BlackNode and the new MapEntry type to ensure they behave identically.

A few bugs were caught in doing so:

  • -nth on RedNodes and BlackNodes with an out of bounds index did not throw an exception.
  • -invoke with an out of bounds index had the same problem
  • -find behaviour on Red and Black nodes did not behave as expected. See CLJS-2014
  • PersistentVectors, RedNodes and BlackNodes all implement IAssociative but missed the -contains-key? method.

The listed bugs were fixed in the patch.

Show
Thomas Mulvaney added a comment - I've attached a patch which introduces the MapEntry type. It's there but not hooked up - PAMs and PHMs don't emit them when you `seq` or `iterate` over them yet. There is a set of tests which check the protocols all map entries should implement: IVector, IAssociative, ISeq etc. The exact same set of tests are run across PersistentVector, RedNode, BlackNode and the new MapEntry type to ensure they behave identically. A few bugs were caught in doing so:
  • -nth on RedNodes and BlackNodes with an out of bounds index did not throw an exception.
  • -invoke with an out of bounds index had the same problem
  • -find behaviour on Red and Black nodes did not behave as expected. See CLJS-2014
  • PersistentVectors, RedNodes and BlackNodes all implement IAssociative but missed the -contains-key? method.
The listed bugs were fixed in the patch.
Hide
David Nolen added a comment -

This patch appears broken. When I try to run the tests I get exception about `cond` having a non-even number of forms.

Show
David Nolen added a comment - This patch appears broken. When I try to run the tests I get exception about `cond` having a non-even number of forms.
Hide
Thomas Mulvaney added a comment -

Yep, looks like I messed that up squashing/cleaning up commits, sorry. Fixed patch attached.

Show
Thomas Mulvaney added a comment - Yep, looks like I messed that up squashing/cleaning up commits, sorry. Fixed patch attached.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: