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
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
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.
"Claus" == Claus Kick claus_kick@web.de 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.
Am 28.09.2008 um 09:52 schrieb Randal L. Schwartz:
"Claus" == Claus Kick claus_kick@web.de 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 -
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 -
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?
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 -
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?
Bert Freudenberg wrote:
Am 28.09.2008 um 09:52 schrieb Randal L. Schwartz:
> "Claus" == Claus Kick claus_kick@web.de 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?
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 -
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@lists.squeakfoundation.org
-
Bert Freudenberg -
Claus Kick -
Mark Volkmann -
merlyn＠stonehenge.com -
nicolas cellier -
Zulq Alam