Here is how one of my methods was pretty printed.
escape: aString "escapes special characters in XML text" | result | result := ''. aString do: [:char | result := result , (char caseOf: { [$<] -> ['<']. [$>] -> ['>']. [$'] -> [''']. [$"] -> ['"']. [$&] -> ['&']} otherwise: [char asString])]. ^ result Yikes! To my beginner eyes, that doesn't look good. What's the deal with the excessive indentation? BTW, comments better ways to do what this code does are welcomed. Could I use collect for this? --- Mark Volkmann _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
An awkward pretty print is a good sign you need to simplify. I would use
another method for the case statement. You couldn't use collect because for each character as you are not collecting characters but strings when you escape. Perhaps something like... escapeString: aString ^ String streamContents: [:aStream | aString do: [:eachCharacter | aStream nextPutAll: (self escapeCharacter: eachCharacter)] escapeCharacter: aCharacter aCharacter = $< ifTrue: [^ '<']. aCharacter = $> ifTrue: [^ '>']. aCharacter = $' ifTrue: [^ ''']. aCharacter = $& ifTrue: [^ '&']. ^ aCharacter asString I use #streamContents: a lot as #, causes a lot of unnecessary Strings to be created and also unnecessary work. I rarely use the pretty printer but instead aim to keep my methods short and simple. Regards, Zulq. Mark Volkmann wrote: > Here is how one of my methods was pretty printed. > > escape: aString > "escapes special characters in XML text" > | result | > result := ''. > aString > do: [:char | result := result > , (char caseOf: { > [$<] -> ['<']. > [$>] -> ['>']. > [$'] -> [''']. > [$"] -> ['"']. > [$&] -> ['&']} > otherwise: [char asString])]. > ^ result > > Yikes! To my beginner eyes, that doesn't look good. What's the deal with > the excessive indentation? > > BTW, comments better ways to do what this code does are welcomed. Could > I use collect for this? > > --- > Mark Volkmann _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
In reply to this post by Mark Volkmann
Mark Volkmann wrote:
> Here is how one of my methods was pretty printed. > > escape: aString > "escapes special characters in XML text" > | result | > result := ''. > aString > do: [:char | result := result > , (char caseOf: { > [$<] -> ['<']. > [$>] -> ['>']. > [$'] -> [''']. > [$"] -> ['"']. > [$&] -> ['&']} > otherwise: [char asString])]. > ^ result > > Yikes! To my beginner eyes, that doesn't look good. What's the deal > with the excessive indentation? > > BTW, comments better ways to do what this code does are welcomed. Could > I use collect for this? You could do it like this: YourClass >> escape: aString "escapes a String" |result| result := ''. aString do:[:char | result := result, char xmlEscaped]. ^result Character >> xmlEscaped ^self caseOf: { [$<] -> ['<']. [$>] -> ['>']. [$'] -> [''']. [$"] -> ['"']. [$&] -> ['&'] } otherwise: [self asString]. The advantage is that your own methos is very short; you hide the actual transformation in another method you do not have to care about for the future. _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
>>>>> "Claus" == Claus Kick <[hidden email]> writes:
Claus> YourClass >> escape: aString Claus> "escapes a String" Claus> |result| Claus> result := ''. Claus> aString do:[:char | result := result, char xmlEscaped]. Claus> ^result This is an expensive way to build a string. #collect: would be better: aString collect: [:char | char xmlEscaped]. Internally, that uses a Stream, which extends itself nicely as new data appears. In your version, the early string data is getting repeatedly copied to make each new string. Ouch. -- Randal L. Schwartz - Stonehenge Consulting Services, Inc. - +1 503 777 0095 <[hidden email]> <URL:http://www.stonehenge.com/merlyn/> Smalltalk/Perl/Unix consulting, Technical writing, Comedy, etc. etc. See http://methodsandmessages.vox.com/ for Smalltalk and Seaside discussion _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
Am 28.09.2008 um 09:52 schrieb Randal L. Schwartz: >>>>>> "Claus" == Claus Kick <[hidden email]> writes: > > Claus> YourClass >> escape: aString > > Claus> "escapes a String" > > Claus> |result| > > Claus> result := ''. > Claus> aString do:[:char | result := result, char xmlEscaped]. > Claus> ^result > > This is an expensive way to build a string. #collect: would be > better: > > aString collect: [:char | char xmlEscaped]. > > Internally, that uses a Stream, which extends itself nicely as new > data > appears. In your version, the early string data is getting > repeatedly copied > to make each new string. Ouch. Err, #collect: constructs a string character by character, not from other strings. Double-ouch ;) - Bert - _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
Am 28.09.2008 um 10:12 schrieb Bert Freudenberg:
> Am 28.09.2008 um 09:52 schrieb Randal L. Schwartz: >> >> This is an expensive way to build a string. #collect: would be >> better: >> >> aString collect: [:char | char xmlEscaped]. >> >> Internally, that uses a Stream, which extends itself nicely as new >> data >> appears. In your version, the early string data is getting >> repeatedly copied >> to make each new string. Ouch. > > > Err, #collect: constructs a string character by character, not from > other strings. Double-ouch ;) Guess I have to provide an alternative: #('&' '&' '<' '<' '>' '>' '''' ''' '"' '"') pairsDo: [:c :s| string := string copyReplaceAll: c with: s]. which is invariant under pretty-printing so must be good ;) (this is the same approach as in String>>asHTML) - Bert - _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
In reply to this post by Bert Freudenberg
Bert Freudenberg wrote:
> > Am 28.09.2008 um 09:52 schrieb Randal L. Schwartz: > >>>>>>> "Claus" == Claus Kick <[hidden email]> writes: >> >> >> Claus> YourClass >> escape: aString >> >> Claus> "escapes a String" >> >> Claus> |result| >> >> Claus> result := ''. >> Claus> aString do:[:char | result := result, char xmlEscaped]. >> Claus> ^result >> >> This is an expensive way to build a string. #collect: would be better: >> >> aString collect: [:char | char xmlEscaped]. I had actually in mind to do it this way - but I thought the do:[] is easier to read. Runtime performance is always step 2 for me. >> Internally, that uses a Stream, which extends itself nicely as new data >> appears. In your version, the early string data is getting >> repeatedly copied >> to make each new string. Ouch. That may be, my Squeak is rusty (VW these days). > Err, #collect: constructs a string character by character, not from > other strings. Double-ouch ;) So, the gist is - whether you do do:[] or collect:[] doesnt matter much? _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
In reply to this post by Bert Freudenberg
Bert Freudenberg wrote:
> Guess I have to provide an alternative: > > #('&' '&' '<' '<' '>' '>' '''' ''' '"' '"') > pairsDo: [:c :s| string := string copyReplaceAll: c with: s]. > (this is the same approach as in String>>asHTML) Ok, to nitpick (;)) one message send less for each pair: #('&' '&' '<' '<' '>' '>' '''' ''' '"' '"') pairsDo: [:c :s| string := string copyReplaceAll: c with: s asTokens:false]. How do pairs compare to dictionaries in Squeak? _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
Am 29.09.2008 um 10:07 schrieb Claus Kick: > Bert Freudenberg wrote: > > > Guess I have to provide an alternative: >> #('&' '&' '<' '<' '>' '>' '''' ''' '"' '"') >> pairsDo: [:c :s| string := string copyReplaceAll: c with: s]. > >> (this is the same approach as in String>>asHTML) > > Ok, to nitpick (;)) one message send less for each pair: > > #('&' '&' '<' '<' '>' '>' '''' ''' '"' '"') > pairsDo: [:c :s| string := string copyReplaceAll: c with: s > asTokens:false]. This is silly advice. Why would you willingly expose an implementation detail? There are many constructs that could be expanded but readability and portability is hurt by that. > How do pairs compare to dictionaries in Squeak? Well, there are no "pairs" per se but just an iteration method on sequenceable collections. When iterating a Dictionary, the order is unpredictable. In my example it is vital to replace '&' first. Also, Squeak does not have a literal syntax for dictionaries but for Arrays so my example is more efficient because the Array is constructed at compile-time. OTOH, in real code one might put the dictionary into a class variable so this wouldn't matter. - Bert - _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
Bert Freudenberg wrote:
> > Am 29.09.2008 um 10:07 schrieb Claus Kick: > >> Bert Freudenberg wrote: >> >> > Guess I have to provide an alternative: >> >>> #('&' '&' '<' '<' '>' '>' '''' ''' '"' '"') >>> pairsDo: [:c :s| string := string copyReplaceAll: c with: s]. >> >> >>> (this is the same approach as in String>>asHTML) >> >> >> Ok, to nitpick (;)) one message send less for each pair: >> >> #('&' '&' '<' '<' '>' '>' '''' ''' '"' '"') >> pairsDo: [:c :s| string := string copyReplaceAll: c with: s >> asTokens:false]. > > > This is silly advice. Why would you willingly expose an implementation > detail? There are many constructs that could be expanded but > readability and portability is hurt by that. Yes, it was mostly silly, thats what the ;) was for. Sometimes, however, if you have lets say an operation going for a million of loops (i.e. recursive checks over huge object groups), a few unnecessary message sends can cost you precious time. We had code like that at my old shop. However mostly, I wanted to ask the following: >> How do pairs compare to dictionaries in Squeak? > > > Well, there are no "pairs" per se but just an iteration method on > sequenceable collections. > > When iterating a Dictionary, the order is unpredictable. In my example > it is vital to replace '&' first. > > Also, Squeak does not have a literal syntax for dictionaries but for > Arrays so my example is more efficient because the Array is constructed > at compile-time. OTOH, in real code one might put the dictionary into > a class variable so this wouldn't matter. Ok, so if you used a class variable then it would not matter. How well are look-ups performing with Squeak? _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
Am 29.09.2008 um 12:11 schrieb Claus Kick:
> How well are look-ups performing with Squeak? Well, measure it. Array indexing is faster because dictionaries are implemented using arrays. - Bert - _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
In reply to this post by Claus Kick
Am 29.09.2008 um 09:52 schrieb Claus Kick: > Bert Freudenberg wrote: >> Am 28.09.2008 um 09:52 schrieb Randal L. Schwartz: >>> aString collect: [:char | char xmlEscaped]. > > I had actually in mind to do it this way - but I thought the do:[] > is easier to read. Runtime performance is always step 2 for me. > >>> Internally, that uses a Stream, which extends itself nicely as >>> new data >>> appears. It does not use a Stream, #collect: knows the size of the collection in advance, it's the same as the original one. You were probably thinking of something like ^String streamContents: [:strm | aString do: [:c | sstrm nextPutAll: char xmlEscaped]] In some images there is #gather: which does this more concisely but unfortunately uses Array instead of "self species" so it would not work with Strings. >>> In your version, the early string data is getting repeatedly >>> copied >>> to make each new string. Ouch. > > That may be, my Squeak is rusty (VW these days). I'm pretty certain it would be the same in VW because #, has to copy (unless you apply serious amounts of magic). >> Err, #collect: constructs a string character by character, not >> from other strings. Double-ouch ;) > > So, the gist is - whether you do do:[] or collect:[] doesnt matter > much? Well, #collect: is much more readable and concise, and might be optimized for some sorts of collections. In general it's good practice to use the most specific iteration method available rather than emulating using #do: (and always think twice, no, three times before using #to:do:). - Bert - _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
Bert Freudenberg a écrit :
> snip... > > Well, #collect: is much more readable and concise, and might be > optimized for some sorts of collections. > > In general it's good practice to use the most specific iteration method > available rather than emulating using #do: (and always think twice, no, > three times before using #to:do:). > > - Bert - Yes, very good hint. Apart for implementing higher level message, there is mostly one reason to use to:do: and it is often a bad one (I abused of, myself): It is used to optimize code with some sort of inlining that can eliminate creation of blocks and squeeze the cost of a few message sends... The danger is to make code inexpressive, longer than necessary, and not extensible to some other collection class. Let us not optimize code prematurely! Nicolas _______________________________________________ Beginners mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/beginners |
Free forum by Nabble | Edit this page |