soaplite-devel

[Soaplite-devel] bug and comments

[Kostas C. <ko...@ch...>]

Hello,

I'd like to report a bug in SOAP::Serializer (appeared, as far as I can
tell, in version 0.65-beta6), that prevents XMLRPC::Serializer from
being subclassed. From SOAP::Serializer::encode_object

  } elsif (UNIVERSAL::isa($object => 'ARRAY')) {
      # Added in SOAP::Lite 0.65_6 to fix an XMLRPC bug
      return $self->encodingStyle eq "" ||
             ref $self eq 'XMLRPC::Serializer' ?
	  $self->encode_array($object, $name, $type, $attr) :
	  $self->encode_literal_array($object, $name, $type, $attr);

if you use My::Serializer instead of XMLRPC::Serializer, then the test
  $self eq 'XMLRPC::Serializer'
will fail and arrays will not be encoded correctly. A simple fix is to
change the test to
  UNIVERSAL::isa($self, 'XMLRPC::Serializer')


Another comment wrt subclassing the serializers: many functions call
new() on an object, due do statements of the form:
  my $self = shift->new;
As a consequence, when you subclass a serializer your constructor is
called multiple times, unless you add a test like
  return $self if ref $self;
This is an unnatural behaviour and should be written in the
documentation. Or better, these statements should be replaced by the
faster and saner:
  my $self = ref $_[0] ? shift : shift->new;
or something similar.


That's all for now. Thanks a lot for this great module. I was also
pleased to see the announcement of 0.70 and the recent activity in the
mailing list.

Kostas

Re: [Soaplite-devel] bug and comments

[Jason S. <jas...@gm...>]

Meant to send this to the list...

---------- Forwarded message ----------
From: Jason Stewart <jas...@gm...>
Date: 29 Feb 2008 08:26
Subject: Re: [Soaplite-devel] bug and comments
To: Kostas Chatzikokolakis <ko...@ch...>


Hey,


 On 29/02/2008, Kostas Chatzikokolakis <ko...@ch...> wrote:

 >  if you use My::Serializer instead of XMLRPC::Serializer, then the test
 >   $self eq 'XMLRPC::Serializer'
 >  will fail and arrays will not be encoded correctly. A simple fix is to
 >  change the test to
 >   UNIVERSAL::isa($self, 'XMLRPC::Serializer')


looks like a bug...


 >  Another comment wrt subclassing the serializers: many functions call
 >  new() on an object, due do statements of the form:
 >   my $self = shift->new;
 >  As a consequence, when you subclass a serializer your constructor is
 >  called multiple times


hmmm...

 I'm not sure what the purpose of that code is, but it looks like a
 typical copy constructor idiom. The method wants to use a temporary
 copy of your object so as not to change the state so it makes a copy
 by calling new() on the object. If that is so, then your solution of
 just returning the object could lead into strange behavior.

 Perhaps Martin or others can clarify.

 Cheers, jas.

Re: [Soaplite-devel] bug and comments

[Martin K. <mar...@fe...>]

Hi Kostas,

I opened a bug report on sf.net and fixed this in SVN.

Thanks for reporting,

Martin

Am Donnerstag, den 28.02.2008, 19:38 +0100 schrieb Kostas
Chatzikokolakis:
> Hello,
> 
> I'd like to report a bug in SOAP::Serializer (appeared, as far as I can
> tell, in version 0.65-beta6), that prevents XMLRPC::Serializer from
> being subclassed. From SOAP::Serializer::encode_object
> 
>   } elsif (UNIVERSAL::isa($object => 'ARRAY')) {
>       # Added in SOAP::Lite 0.65_6 to fix an XMLRPC bug
>       return $self->encodingStyle eq "" ||
>              ref $self eq 'XMLRPC::Serializer' ?
> 	  $self->encode_array($object, $name, $type, $attr) :
> 	  $self->encode_literal_array($object, $name, $type, $attr);
> 
> if you use My::Serializer instead of XMLRPC::Serializer, then the test
>   $self eq 'XMLRPC::Serializer'
> will fail and arrays will not be encoded correctly. A simple fix is to
> change the test to
>   UNIVERSAL::isa($self, 'XMLRPC::Serializer')
> 
> 
> Another comment wrt subclassing the serializers: many functions call
> new() on an object, due do statements of the form:
>   my $self = shift->new;
> As a consequence, when you subclass a serializer your constructor is
> called multiple times, unless you add a test like
>   return $self if ref $self;
> This is an unnatural behaviour and should be written in the
> documentation. Or better, these statements should be replaced by the
> faster and saner:
>   my $self = ref $_[0] ? shift : shift->new;
> or something similar.
> 
> 
> That's all for now. Thanks a lot for this great module. I was also
> pleased to see the announcement of 0.70 and the recent activity in the
> mailing list.
> 
> Kostas
> 
> 
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> Soaplite-devel mailing list
> Soa...@li...
> https://lists.sourceforge.net/lists/listinfo/soaplite-devel
>