code review 7486049: doc/effective_go.html: unify and expand the discussion ...

doc/effective_go.html: unify and expand the discussion of Sprintf and String
It's a common mistake to build a recursive String method; explain it well and
show how to avoid it.

[r, 2013-03-06 18:56:16]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2013-03-06 19:18:35]

LGTM

On Wed, Mar 6, 2013 at 10:56 AM, <r@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> doc/effective_go.html: unify and expand the discussion of Sprintf and
> String
> It's a common mistake to build a recursive String method; explain it
> well and
> show how to avoid it.
>
> Please review this at
https://codereview.appspot.**com/7486049/<https://codereview.appspot.com/7486...
>
> Affected files:
>   M doc/effective_go.html
>
>
> Index: doc/effective_go.html
> ==============================**==============================**=======
> --- a/doc/effective_go.html
> +++ b/doc/effective_go.html
> @@ -1712,8 +1712,40 @@
>  </p>
>  <p>
>  Our <code>String</code> method is able to call <code>Sprintf</code>
> because the
> -print routines are fully reentrant and can be used recursively.
> -We can even go one step further and pass a print routine's arguments
> directly to another such routine.
> +print routines are fully reentrant and can be wrapped this way.
> +There is one important detail to understand, however: don't construct a
> <code>String</code>
> +method by calling <code>Sprintf</code> in a way that will recur into your
> +<code>String</code> method indefinitely.
> +This can happen if the <code>Sprintf</code> call attempts to print the
> receiver directly as
> +a string, which in turn will invoke the method again.
> +It's a common and easy mistake to make, as this example shows.
> +</p>
> +
> +<pre>
> +type MyString string
> +func (m MyString) String() string {
> +    return fmt.Sprintf("MyString=%s", m) // Error: will recur forever.
> +}
> +</pre>
> +
> +<p>
> +It's also easy to fix: convert the argument to the basic string type,
> which does not have the
> +method.
> +</p>
> +
> +<pre>
> +type MyString string
> +func (m MyString) String() string {
> +    return fmt.Sprintf("MyString=%s", string(m)) // OK: note conversion.
> +}
> +</pre>
> +
> +<p>
> +In the <a href="#initialization">**initialization section</a> we'll see
> another technique that avoids this recursion.
> +</p>
> +
> +<p>
> +Another printing technique is to pass a print routine's arguments
> directly to another such routine.
>  The signature of <code>Printf</code> uses the type
> <code>...interface{}</code>
>  for its final argument to specify that an arbitrary number of parameters
> (of arbitrary type)
>  can appear after the format.
> @@ -1857,13 +1889,13 @@
>  </p>
>
>  <p>
> -Note that it's fine to call <code>Sprintf</code> and friends in the
> -implementation of <code>String</code> methods, but beware of
> -recurring into the <code>String</code> method through the nested
> -<code>Sprintf</code> call using a string format
> -(<code>%s</code>, <code>%q</code>, <code>%v</code>, <code>%x</code> or
> <code>%X</code>).
> -The <code>ByteSize</code> implementation of <code>String</code> is safe
> -because it calls <code>Sprintf</code> with <code>%f</code>.
> +The use here of <code>Sprintf</code>
> +to implement <code>ByteSize</code>'s <code>String</code> method is safe
> +(avoids recurring indefinitely) not because of a conversion but
> +because it calls <code>Sprintf</code> with <code>%f</code>,
> +which is not a string format: <code>Sprintf</code> will only call
> +the <code>String</code> method when it wants a string, and <code>%f</code>
> +wants a floating-point value.
>  </p>
>
>  <h3 id="variables">Variables</h3>
> @@ -2022,10 +2054,8 @@
>  }
>  </pre>
>  <p>
> -The conversion causes <code>s</code> to be treated as an ordinary slice
> -and therefore receive the default formatting.
> -Without the conversion, <code>Sprint</code> would find the
> -<code>String</code> method of <code>Sequence</code> and recur
> indefinitely.
> +This method is another example of the conversion technique for calling
> +<code>Sprintf</code> safely from a <code>String</code> method.
>  Because the two types (<code>Sequence</code> and <code>[]int</code>)
>  are the same if we ignore the type name, it's legal to convert between
> them.
>  The conversion doesn't create a new value, it just temporarily acts
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[adg, 2013-03-06 23:35:37]

https://codereview.appspot.com/7486049/diff/1/doc/effective_go.html
File doc/effective_go.html (right):

https://codereview.appspot.com/7486049/diff/1/doc/effective_go.html#newcode1716
doc/effective_go.html:1716: There is one important detail to understand,
however: don't construct a <code>String</code>
should this sentence begin a new paragraph? (a paragraph about the important
detail)

https://codereview.appspot.com/7486049/diff/1/doc/effective_go.html#newcode1726
doc/effective_go.html:1726: func (m MyString) String() string {
put a blank line between the two declarations; that's the gofmt style

[adg, 2013-03-06 23:46:09]

LGTM after those fixes

[r, 2013-03-06 23:47:54]

*** Submitted as https://code.google.com/p/go/source/detail?r=d7af5bb3b6ca ***

doc/effective_go.html: unify and expand the discussion of Sprintf and String
It's a common mistake to build a recursive String method; explain it well and
show how to avoid it.

R=golang-dev, bradfitz, adg
CC=golang-dev
https://codereview.appspot.com/7486049