Skip to content

Make JObject and JArray immutable #32

Open
@non

Description

Currently (and historically) JObject and JArray are mutable. They are case classes wrapping mutable data structures.

@ijuma wisely points out that this is maybe not the best for folks who want to use long-lived JObject instances, and suggests it would be nice to make the types safer.

Here are some possible scenarios (from least to most safe):

(I) Types are constructed with mutable collections, and expose them via getters. This is the status quo.

(II) Types are still constructed with mutable collections, but don't expose them directly. Data is accessed via read-only methods, or by copying the underlying data into a new structure. Mostly safe, although a fiendish user could construct a JObject, hang onto the mutable map, and change it later. (The parser is guaranteed not to do this.) The downside here is that it becomes more expensive to use JObject and JArray as a scratchpad during parse/modify/save cycles. (This is probably not a big deal.)

(III) Provide a public constructor using an immutable map, and a private constructor used internally that uses mutable maps. This way, users can't play games with mutable data, and (assuming the parser doesn't do anything fishy) we are totally safe. The downside here is that the objects become more expensive to construct, or that we have to add different JObject types for the different backing structures.

(IV) Convert Jawn to use immutable data structures only, possibly using builders in the Parser. This solves the safety issues, but probably increases the memory footprint and might have other performance issues.

Personally I think I prefer (II), but I'm open to hearing other suggestions.

Activity

added a commit that references this issue on May 27, 2015
ijuma

ijuma commented on May 27, 2015

@ijuma

(II) is good for me too.

non

non commented on May 27, 2015

@non
ContributorAuthor

Great. I'll try to make this change and release it, probably as 0.9.0 due to the API change.

ijuma

ijuma commented on May 28, 2015

@ijuma

While you look at this, I think it may be worth thinking about how users will iterate over the fields of a JObject or the elements of an array.

Unless I missed something, the way to do that currently is to use pattern matching in order to get access to the mutable vs. In the new version, one would have access via getters only. This works, but it's a bit verbose. What are your thoughts regarding methods like getJObject, asJObject and getJArray and asJArray? Again, these can be done easily as extension methods if the desire is to keep the AST sugar-free. :)

non

non commented on May 28, 2015

@non
ContributorAuthor

I do want to keep things simple. I'm imagining implementing a few methods like .iterator, .size, .isEmpty, etc. and then maybe having the classes extend Iterable[_].

This would mean that more complicated methods like .map and .filter might need to be added by someone else.

reopened this on May 28, 2015
non

non commented on May 28, 2015

@non
ContributorAuthor

(Accidentally mis-clicked and closed the issues temporarily.)

non

non commented on Jun 5, 2015

@non
ContributorAuthor

So, after a bit of thought, I think I will add the following methods to JValue:

// for numbers
def asInt: Int

// for objects and arrays
def size: Int
def isEmpty: Boolean
def nonEmpty: Boolean

// for arrays
def elements: Iterator[JValue]
def asVector: Vector[JValue]

// for objects
def items: Iterator[(String, JValue)] = Iterator.empty
def asMap: Map[String, JValue]

How does this sound @ijuma?

non

non commented on Jun 5, 2015

@non
ContributorAuthor

Oh, and I might also add:

def orElse(jv: => JValue): JValue = if (this == JNull) jv else this

Since a lot of the API returns JNull as a failure case, I think this would be very useful.

ijuma

ijuma commented on Jun 7, 2015

@ijuma

Sounds good to me (including orElse).

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

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Make JObject and JArray immutable · Issue #32 · typelevel/jawn