pretty printing

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

pretty printing

Mark Volkmann
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: {
                                                                [$<] -> ['&lt;'].
                                                                [$>] -> ['&gt;'].
                                                                [$'] -> ['&apos;'].
                                                                [$"] -> ['&quot;'].
                                                                [$&] -> ['&amp;']}
                                                                 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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

Zulq Alam-2
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: [^ '&lt;'].
    aCharacter = $> ifTrue: [^ '&gt;'].
    aCharacter = $' ifTrue: [^ '&apos;'].
    aCharacter = $& ifTrue: [^ '&amp;'].
    ^ 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: {
>                                 [$<] -> ['&lt;'].
>                                 [$>] -> ['&gt;'].
>                                 [$'] -> ['&apos;'].
>                                 [$"] -> ['&quot;'].
>                                 [$&] -> ['&amp;']}
>                                  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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

Claus Kick
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: {
>                                 [$<] -> ['&lt;'].
>                                 [$>] -> ['&gt;'].
>                                 [$'] -> ['&apos;'].
>                                 [$"] -> ['&quot;'].
>                                 [$&] -> ['&amp;']}
>                                  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:
{
        [$<] -> ['&lt;'].
        [$>] -> ['&gt;'].
        [$'] -> ['&apos;'].
        [$"] -> ['&quot;'].
        [$&] -> ['&amp;']
}
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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

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.

--
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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

Bert Freudenberg

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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

Bert Freudenberg
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:

#('&' '&amp;' '<' '&lt;' '>' '&gt;' '''' '&apos;' '"' '&quot;')
        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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

Claus Kick
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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

Claus Kick
In reply to this post by Bert Freudenberg
Bert Freudenberg wrote:

  > Guess I have to provide an alternative:
>
> #('&' '&amp;' '<' '&lt;' '>' '&gt;' '''' '&apos;' '"' '&quot;')
>     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:

#('&' '&amp;' '<' '&lt;' '>' '&gt;' '''' '&apos;' '"' '&quot;')
  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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

Bert Freudenberg

Am 29.09.2008 um 10:07 schrieb Claus Kick:

> Bert Freudenberg wrote:
>
> > Guess I have to provide an alternative:
>> #('&' '&amp;' '<' '&lt;' '>' '&gt;' '''' '&apos;' '"' '&quot;')
>>    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:
>
> #('&' '&amp;' '<' '&lt;' '>' '&gt;' '''' '&apos;' '"' '&quot;')
> 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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

Claus Kick
Bert Freudenberg wrote:

>
> Am 29.09.2008 um 10:07 schrieb Claus Kick:
>
>> Bert Freudenberg wrote:
>>
>> > Guess I have to provide an alternative:
>>
>>> #('&' '&amp;' '<' '&lt;' '>' '&gt;' '''' '&apos;' '"' '&quot;')
>>>    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:
>>
>> #('&' '&amp;' '<' '&lt;' '>' '&gt;' '''' '&apos;' '"' '&quot;')
>> 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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

Bert Freudenberg
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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

Bert Freudenberg
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
Reply | Threaded
Open this post in threaded view
|

Re: pretty printing

Nicolas Cellier-3
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