svenvc / ston

STON - Smalltalk Object Notation - A lightweight text-based, human-readable data interchange format for class-based object-oriented languages like Smalltalk.
MIT License
135 stars 32 forks source link

STON incorrectly converts instances of Interval #5

Closed dalehenrich closed 8 years ago

dalehenrich commented 11 years ago

Admittedly I have not updated to the latest version of STON, so if it this is fixed great!

Otherwise, it appears that STON does not handle Interval instances correctly as the following throws an error in Pharo1.4:

...Smalltalk STON fromString: (STON toString: (3 to: 6)) ...

Apparently STON turns an interval into a collection: ...Smalltalk 'Interval[3,4,5,6]' ... which doesn't seem correct ...

Dale

svenvc commented 11 years ago

Dale,

Yes this is a problem, thanks for reporting it. However, it is in the same league as the Text issue.

STON is very simple: all Collections with the exception of Dictionaries, String, Symbol and ByteArray are treated the same: their elements are written out as a list and read in as a list whose elements are added one by one to a new instance.

I absolutely agree that this is too simplistic in a number of cases, although it works for the vast majority of normal/regular Collection subclasses.

From your reports, I would infer that you expect STON to be able to serialize a lot of Smalltalk classes just like that. Can you describe your use case ?

Because no matter what we do, for example, the sortBlock in SortedCollection will never be serializable, and there will be many other examples.

Sven

dalehenrich commented 11 years ago

Sven,

I suppose I'm reporting bugs when I run into a situation where I'm using STON and hit a walkback ... in my application I simply stopped using the Interval ...

I understand that the nature of STON (and every other serialization format) that special cases exist and must be dealt with, so consider these reports as pointers to additional special cases.

I suppose I do have the expectation that STON would either handle the "standard set of Smalltalk classes" or give an error message when an attempt is made to serialize a class that isn't handled ...

With that said, I don't hold you responsible for doing this ... again I consider the issue list a place holder for future development ... in my work I will be making a pass on STON at some point in time in the near future when I integrate your recent changes into the GemStone versions for other platforms that I'm maintaining and at that point I will scan the issue list for things that I could address ... this issue list is also useful for setting expectations of other users of STON as to what is and is not expected to work ...

As to my use case, I am working on a remote development environment for GemStone using Pharo as the client (a replacement for GemTools). The client is a relatively thin client, so I'm not expecting arbitrary GemStone classes to be serialized to Pharo ... for the most part I am shipping custom GUI state objects across the wire (in both directions) and the other day I happened to find it convenient to use an Interval (a standard Smalltalk class) to represent the selection interval for a text window ... the text morph expects an Interval ... when I hit the STON issue, I simply started shipping the begin/end points of the Interval, constructing the interval on the fly.

Despite these issue reports, I am finding STON very useful!

Dale

svenvc commented 8 years ago

Interval is a PIA, it is more like a fake collection. I will consider adding a special case to not treat it like a collection. Maybe there should be some easy mechanism to add exceptions.

dalehenrich commented 8 years ago

Yes, it is worth waiting and letting the problem marinate when you hit these odd corner cases ... the "best" solution is not always obvious ...

Does it seem that in these exceptional cases the "fallback" is to simply record and pass along the state? Interval is a subclass of SequenceableCollection for behavior reasons not state ...

svenvc commented 8 years ago

I added support for Interval upstream, will arrive later in git