Immutable API?

[deathcap]

Immutability has many benefits, making it easier to reason about state avoiding certain classes of bugs. Noteworthy for itempile:

• Empty slots & illegal state: count<=0 is not usually a legitimate value, but it can arise easily during splitting, merging, decreasing. By convention, the inventory module uses 'undefined' to represent empty slots instead.

However, this convention requires special-cases empty slots accordingly, on both input (checking for undefined) and output (setting undefined if an operation empties a slot). If itempiles were immutable, instead of mutating in-place (requiring the caller to check in/out for undefined), operations would return a new object, which could either be an itempile instance or undefined, therefore simplifying and centralizing the "no item" handling logic.

• Unintentional references & cloning: if a copy of an object is desired, simply assigning it with = is insufficient since modifying it will modify the original.

This is why 0.0.3 added clone(), to create a new pile with the same item, count, tags, yet a distinct object that can be modified independently. Currently itempile clone is only used in craftingrecipes computeOutput(), since it returns a new pile based on a recipe index.

The clone() implementation in itempile 0.0.3 is actually wrong, since it doesn't clone the tags (edit: fixed in 1ea6fd1). Tags can be arbitrarily nested so I'm going to need a "deep clone" to properly fix this.

Or if everything was immutable, no cloning would be needed since nothing could change.

Downsides of immutability:

• Incompatibility: need to update all usages of itempile to new API (minor)
• Somewhat more difficult to use: methods need to return the new object, but may also need to return other values (almost always).

In CoffeeScript this is painless with [a2, b] = a.mergedPile(b), destructuring assignment, but is much more cumbersome in current widely supported JavaScript implementations. Firefox 2 with JavaScript 1.7 added destructuring assignment, and supposedly it is slated for ECMAScript 6, but the current stable version of Chrome (31.0.1650.63) doesn't support it. This means you have to write something like: var array = a.mergedPile(b); var a2 = array[0], b = array[1], not pleasant at all. It is only syntax, but syntax matters, still a pain.

• Performance: unknown. But it is a concern when creating new objects everywhere.

In summary, immutability for this module may or may not be worth it. Definitely an interesting idea though.

[deathcap]

Added on branch 'immutable', but not sure if I'll use it (an enjoyable experiment nonetheless)