From: Robert Dober <robert.dober@...>
Date: 2008-12-13T05:26:39+09:00
Subject: Re: [SUMMARY] AnsiString (#185)

On Fri, Dec 12, 2008 at 7:06 PM, Matthew Moss <matt@moss.name> wrote:
Since this quiz doesn't use the Module
> mechanism in Robert's `register_lib` routine, I've removed the related
> references for clarity. I suspect those are for a larger set of library
> management routines.)
Exactly, it was the multi library approach which interested me more
than the ANSIString implementation, hence
the sloppy implementation :(.
<snip>
>
> Let's look at string concatenation:
>
>        class ANSIString
>          def + other
>            other.add_reverse self
>          rescue NoMethodError
>            self.class::new( *( __end__ << other ) )
>          end
>
>          def add_reverse an_ansi_str
>            self.class::new( *(
>              an_ansi_str.send( :__end__ ) + __end__
>            ) )
>          end
>
>          private
>          def __end__
>            @strings.reverse.find{ |x| Symbol === x} == :end ?
>              @strings.dup : @strings.dup << :end
>          end
>        end
>
> Before we get to the concatenation itself, take a quick look at helper
> method `__end__`. It looks for the last symbol and compares it against
> `:end`. Whether true or false, the `@string` array is duplicated (and so
> protects the instance variable from change). Only, `__end__` does not append
> another `:end` symbol if unnecessary.
>
> I was a little confused, at first, about the implementation of `ANSIString`
> concatenation. Perhaps Robert had other plans in mind, but it seemed to me
> this work could be simplified. Since `add_reverse` is called nowhere else
> (and I couldn't imagine it being called by the user, despite the public
> interface), I tried inserting `add_reverse` inline to `+` (fixing names
> along the way):
>
>        def + other
>      other.class::new( *(self.send(:__end__) + other.__end__) )
>        rescue NoMethodError
>          self.class::new( *( __end__ << other ) )
>        end
>
> And, with further simplification:
>
>        def + other
>          other.class::new( *( __end__ + other.send(:__end__) ) )
>        rescue NoMethodError
>          self.class::new( *( __end__ << other ) )
>        end
>
> I believed Robert had a bug, neglecting to call `__end__` in the second
> case, until I realized my mistake: `other` is not necessarily of the
> `ANSIString` class, and so would not have the `__end__` method. My attempt
> to fix my mistake was to rewrite again as this:
>
>        def + other
>          ANSIString::new( *( __end__ + other.to_s ) )
>        end
>
> But that has its own problems if `other` *is* an `ANSIString`; it neglects
> to end the string and converts it to a simple `String` rather than
> maintaining its components. Clearly undesirable. Obviously, Robert's
> implementation is the right way... or is it? Going back to this version:
>
>        def + other
>          other.class::new( *( __end__ + other.send(:__end__) ) )
>        rescue NoMethodError
>          self.class::new( *( __end__ << other ) )
>        end
>
> Ignoring the redundancy, this actually works. My simplification will throw
> the `NoMethodError` exception, because `String` does not define `__end__`,
> just as Robert's version throws that exception if either `add_reverse` or
> `__end__` is not defined. So, removing redundancy, I believe concatenation
> can be simplified correctly as:
>
>        def + other
>                self.class::new( *(
>                        __end__ + (other.send(:__end__) rescue [other] )
>                ) )
>        end
>
> For me, this reduces concatenation to something more quickly understandable.
>
> One last point on concatenation; Robert's version will create an object of
> class `other.class` if that class has both methods `add_reverse` and
> `__end__`, whereas my simplification does not. However, it seems unlikely to
> me that any class other than `ANSIString` will have those methods. I
> recognize that my assumption here may be flawed; Robert will have to provide
> further details on his reasoning or other uses of the code.
Not really I was quite sloppy, it took me same time to re-understand
my code, always a bad sign.
Sorry for giving you so much work :(.
>
<snip>
Cheers
R.