'Path' implementation

[Brian T. Rice]

Hi Pavel,

Thanks for taking the time to work on these. All of the changes (and 
tests!) look good, and I'll apply them now.

Pavel Holejsovsky wrote:
> Hi,
> 
> This is my attempt at finishing implementation of 'src/lib/path.slate'. 
>    The patch contains also some basic unit tests, so I believe it is not 
> completely broken...  Any comments and corrections are greatly 
> appreciated - I'm still trying to find my 'path' through this twisty 
> little maze of objects, all alike ;-)
> 
> Summary of problems that I met during the implementation follows.  Any 
> comments from anyone will be very welcome ... thanks
> 
> ***********************************************************************
> First difficulty I met was 'Types Type rules' object.  This beast 
> doesn't have any 'traits' slot (yuck), and therefore (as the comment in 
> src/mobius/types.slate says), all methods must be dispatched by sendTo: 
> instead of direct calls.  This means that it is not even possible to 
> insert it into IdentitySet etc.  I did not want to 'pollute' the 
> implementation of the Path by using sendTo: instead of normal method 
> calls, so I closed the appropriate code in "[] breakOn: MethodNotFound" 
> envelope and it seems to work - 'Types Type rules' is simply ignored 
> when searching for path.  Is there any more elegant solution for this?

As far as I am aware, there isn't. Perhaps Lee knows of a better 
solution, or perhaps an idea of a better way to handle type objects and 
derivation.

> ***********************************************************************
> I tried to use ({#a -> #b} as: Dictionary) idiom, but it did not work - 
> silently creating bogus dictionaries.  I believe I can fix this (by 
> implementing special case of Dictionary WriteStream, I think), but I'm 
> not sure if this should work at all (conceptually).

I think it's relevant idea, but there's obviously a missing method and 
whatever method is getting applied needs to be overridden at the very 
least. Silent failures (without even a comment on the method) like that 
should just not happen. I'll take a look.

> ***********************************************************************
> hash/= methods - I thought that 'hash' *must* return the same value for 
> any two objects for which '=' returns True.  But:
> 
>  > addSlot: #a1 valued: {0}.
> {0}
>  > addSlot: #a2 valued: {0}.
> {0}
>  > a1 = a2.
> True
>  > a1 hash.
> 102089623
>  > a2 hash.
> 127015919
> 
> Is this complete misconception on my side, or some problem in current 
> slate implementation?

This is very curious. The bit patterns for SmallIntegers should be used 
directly for the hash (a primitive method), and should not be different 
for the same value. This is a bug and we'll try to fix it.

> ***********************************************************************
> There is something fishy with ExtensibleArray.  Following one-liner IMHO 
> should work, but:
> 
>  > ExtensibleArray newEmpty writer ; #a.
> The following condition was signaled:
> 0 is not a key in {}
> 
> I tried to track this down a bit; the condition is signalled from 
> iterator.slate, pastEndPut: method;
> 
> 1: ws@(Sequence WriteStream traits) pastEndPut: obj
> 2: [| c cs |
> 3:   c: ws collection.
> 4:   ws collection: c ; (c newSize: ((c size max: 20) min: 1000000)).
> 5:   ws writeLimit: ws collection size.
> 6:   ws collection at: ws position put: obj.
> 7:   ws position: ws position + 1.
> 8:   obj
> 9: ].
> 
> The problem seems to come from the fact that collection concatenation on 
> line 4 creates ExtensibleArray with 'capacity' based on the 'size' of 
> concatenated arrays, and later line 6 tries to write into non-existent 
> element of the array.  It can be demostrated like this:
> 
> Slate 19> addSlot: #a3 valued: (ExtensibleArray newSize: 5).
> (a1 . Types . a2 . prototypes .
>     VM . globals . Mixins . a3 .
>     traits )
>  > a3 size.
> 0
>  > a3 capacity.
> 5
>  > (a3 ; a3) size.
> 0
>  > (a3 ; a3) capacity.
> 0
> 
> I'm not sure what should be done with these, because looking closer at 
> ExtensibleArray, I found that I have hard time understanding 
> capacity/size idioms; newSize: does not set size but capacity instead, 
> size can be changed only by addFirst:/addLast:/removeFirst:/removeLast:, 
>  at: and at:put: don't care about size at all...  Is this all intentional?

Nope, in fact this method should be overridden to do the obvious 
addLast: thing. All of this complexity is just inherited by (my) 
laziness from Smalltalk's way of doing streams.

The issue with the capacity/size is also inherited from Smalltalk, 
although they use new: instead of newSize:, so in this case Slate is 
slightly confusing. I have been strongly considering merging 
newEmpty/newSize: into new &capacity: which would alleviate a lot of 
this but introduce many sends-with-optionals.

> thanks (mainly for slate, the more I use it the more I like it!), and I 
> apologize for such a long posting.

Don't apologize, this is great work in tracking down problems, and of 
course we're glad that any users are happy. :)

> Pavouk
-------------- next part --------------
A non-text attachment was scrubbed...
Name: water.vcf
Type: text/x-vcard
Size: 208 bytes
Desc: not available
Url : /archives/slate/attachments/20040908/8aad62b3/water.vcf