Re: svn commit: r573017 [1/2] - in /incubator/cxf/trunk: api/src/main/java/org/apache/cxf/interceptor/ api/src/main/java/org/apache/cxf/phase/ common/common/src/main/java/org/apache/cxf/common/commands/ common/common/src/main/java/org/apache/cxf/common/log...

[Daniel Kulp <da...@iona.com>]

On Wednesday 05 September 2007, Glen Mazza wrote:
> > +        if (detail == null) {
> > +            Document d = DOMUtils.createDocument();
> > +            Element element = d.createElement("Fault");
> > +            this.detail = element;
>
> Perhaps the "element" variable can be removed here.

Yep.  Definitely could be.   I'll do it.  


> > +    protected Fault createFault(Throwable ex) {
> > +        //map the JAX-WS faults
> > +        if (ex instanceof SOAPFaultException) {
> > +            SOAPFaultException sfe = (SOAPFaultException)ex;
> > +            SoapFault fault = new
> > SoapFault(sfe.getFault().getFaultString(), +                        
> >                    sfe,
> > +                                           
> > sfe.getFault().getFaultCodeAsQName());
>
> This seems suboptimal, given that SoapFault already works with a
> SOAPFaultException (2nd argument), 

The second arguement is "Throwable".    Inside the CXF SoapFault, it 
pretty much just passes that as the "cause" to the super.

> it should be able to derive the 
> first and third arguments, correct?  I.e., would it be better for
> SoapFault have a constructor that takes just an SOAPFaultException and
> let it internally determine those first and third arguments?

Well, no.   SoapFault is defined in the soap binding.   It's specific to 
our soap implementation.    SOAPFaultException is a JAX-WS specific 
thing and thus should be limitted to just the jax-ws frontend.    The 
mapping to/from the JAX-WS things to the CXF things needs to be limitted 
to the JAX-WS frontend.


Thanks!
-- 
J. Daniel Kulp
Principal Engineer
IONA
P: 781-902-8727    C: 508-380-7194
daniel.kulp@iona.com
http://www.dankulp.com/blog