[Seaside-dev] Issue 293 in seaside: Refactor WAResponse and subclasses to encapsulate stream and use WriteStream instead of ReadWriteStream

[codesite-noreply at google.com]

Updates:
	Status: New
	Owner: ---

Comment #6 on issue 293 by WeybridgeWay: Refactor WAResponse and subclasses  
to encapsulate stream and use WriteStream instead of ReadWriteStream
http://code.google.com/p/seaside/issues/detail?id=293

Lukas,

I've tried to respond to your questions...

- How did you do it before, because "WAPlatform current readWriteStream" is  
part of the platform support.
Furthermore ReadWriteStream IS PART of the ANSI standard (5.9.7, Page 285).

You're right, ReadWriteStream is part of ANSI. I missed that. As to how  
things were working before, I'm not sure. I'm trying to get Seaside 2.9 to  
run and there are a
number of differences. Is there a reason we can't just use a WriteStream  
here? Why is a ReadWriteStream needed?

- The change regarding to WABufferedResponse>>#writeHeadersOn: Your change  
makes the code very
inefficient, because it creates a copy of the complete response body just  
to determine the size of the
contents. Wouldn't it be more reasonable if you implemented #size in Stream?

Stream>>#size is not part ANSI, and we seem to have a problem with our  
implementation. I'll see if we can fix our side.

- The same for "nextPut: Character cr; nextPut: Character lf". They are  
measurably slow, especially on Squeak
when called often. I suggest that we replace occurrences everywhere by  
calling #cr (part of ANSI) and #lf
(would be an ANSI extension).

Unfortunately, while Stream>>#cr is part of ANSI, it allows an  
implementation-defined end-of-line sequence. Given GemStone's UNIX  
background, #cr is implemented
to add an LF!

- The #stream accessor is an integral part of the WAStreamedResponse. It  
cannot be removed. It is used to
retrieve the body-stream and flush the header fields to the socket.  
Furthermore this accessor is used to
create the document where the components are being rendered upon.

I guess I missed that. I expected that WAStreamedResponse would be  
polymorphic with WABufferedResponse and everything not in the WAResponse  
protocol should
be private. I changed the header fields writing and the document creation  
method and all the tests passed. In your checkin for  
Seaside-Core-lr.412.mcz you said that
there were "changes in the WAResponseHierarchy that broke Seaside in  
Pharo/Squeak". I assume that this was something subsequent to Seaside-Core.  
Might those
callers instead use methods on WAResponse so that they don't rely so  
heavily on particular implementation details?

- In the WAResponse #nextPutAll: implementations you call #asString. This  
is a non portable method. We
should use #seasideString in such cases, but I wonder why this is needed  
anyway? It is a potential
performance and memory leak. The stream is supposed to be a character  
stream and in low level code like this
we don't want to do unnecessary copie.

You are right, I should not have used #asString. The reason it was needed  
was because some tests were putting bytes (an instance of ByteArray) on the  
stream.

- Concerning #lf. I just learned that no Smalltalk has such a method, not  
even Squeak. So I suggest that you add #crlf to WriteStream. Most other  
Smalltalks have that.

Actually, most have Character class>>#lf (which is what I was using), but I  
can add WriteStream>>#crlf to GemStone.

James Foster

--
You received this message because you are listed in the owner
or CC fields of this issue, or because you starred this issue.
You may adjust your issue notification preferences at:
http://code.google.com/hosting/settings