Serializing & materializing a Bag with duplicate elements fails about ston HOT 7 CLOSED

Yes I confirm the problem, here is an even shorter version:

STON toString: (Bag withAll: { 1@2. 1@2. 1@2 }).
STON fromString: 'Bag [ Point [ 1, 2 ], @2, @2 ]'.

I know the cause, I just have to think a bit about it.

from ston.

6a894a4

should solve your issue.

thanks again for reporting it.

from ston.

Thanks! That seems to work. Two observations though:

• This makes the notation for Bag more implementation-dependent (the GemStone implementation of Bag has different instance variables than the Pharo one)
• Bags serialized by the previous version of STON can not be materialized by this version anymore (STON fromString: 'Bag[''abc'']' no longer works)

An alternative solution I had in mind is to postpone adding the elements to the Bag to happen after the reference resolution. When creating the Bag, put its elements in an OrderedCollection that serves as a “placeholder”:

Bag class>>fromSton: stonReader

	| elementsPlaceholder collection |
	elementsPlaceholder := OrderedCollection new.
	collection := self with: elementsPlaceholder.
	stonReader parseListDo: [ :each |
		elementsPlaceholder add: each ].
	^ collection

Then in #stonProcessSubObjects:, add the elements to the Bag after resolving their references:

Bag>>stonProcessSubObjects: block

	| elementsPlaceholder |
	elementsPlaceholder := self anyOne.
	self removeAll.
	elementsPlaceholder do: [ :element |
		self add: (block value: element) ].

This also makes the #testCollections2 given above pass. As a sanity-check, it's also able to read a self-referencing Bag (b := STON fromString: 'Bag[@1]'. b anyOne == b ⇒ true) (this wouldn't be the case, I think, if the reference-resolution were done in #fromSton:, which is what I initially had in mind). What does not work yet is reading a Bag without any references, because in that case #stonProcessSubObjects: is simply not sent; I'm not sure yet how best to handle that.

from ston.

By design and by nature, a serialisation format like STON exposes the internal structure of objects. It is just for a limited number of classes that a reasonably abstract and more implementation independent representation can be chosen.

For this reason, I do not consider cross-platform or inter-version compatibility as real goals. But of course I will try to maintain it.

With that being said, I am considering using another custom representation of Bags, with the value->occurences dictionary directly exposed, like

Bag { #a->2. #b->3 }

Would that work on Gemstone ?

BTW, I consider the old representation

 Bag [ #a, #a, #b, #b, #b ]

as almost a bug, since it explodes the Bag's size.

from ston.

Ok, having a custom representation for Bags that avoids the size explosion and works across Smalltalk dialects seems like a good idea. (The GemStone implementation of Bag has more and different instance variables than the Pharo one: Bag allInstVarNames ⇒ #(#'_varyingSize' #'_numEntries' #'_indexedPaths' #'_levels' #'dict' #'size'), dict is like contents in Pharo, size is the size of the Bag; the other variables are inherited from UnorderedCollection).

from ston.

OK, done.

c8cfdda

I also updated https://github.com/svenvc/ston/blob/master/ston-spec.md with a subsection about collections and Bag.

There are some more unit tests too

from ston.

Ok, thanks! I think we can consider this issue as closed.

from ston.