Comment 21 for bug 1241902

Hi Johan, sorry for the problem, we have a double-bounce configuration
in our mail server and perhaps is for that, need to see the logs...

This weekend change the code and send with an alternate mail.

Good weekend.

El mié, 15-01-2014 a las 22:26 +0000, Johan Engelen escribió:
> Hi Jabier,
> Your mail provider is blocking mine, so I'll write my quick answer here:
>
> Hi Jabiertxo,
> quick reply, I have not yet look at your new diff.
>
> I think it is much better if
>
> apply_lpe_to_mask
>
> and
>
> apply_lpe_to_clippath
>
> are *not* member functions. They are not part of the class logically, so
> should not be in there. (note: in C++ it is not the case that every
> function should be in a class, on the contrary it is often much better
> if they are not in the class definition!)
>
>
> On 14-1-2014 10:41, Jabiertxo Arraiza Cenoz wrote:
> > Hi Johan.
> > First thanks for your time, i comment the code a few:
> > Not sure is ok, but finaly, i create two functions for handle most of
> > duplicate content. Both are in sp-clippath.cpp and in sp-mask.cpp, the
> > functions are very similar and maybe is beter use only one in, for
> > example sp-path.cpp.
> >
> > Another cuestion is this duplicate code:
> > ===================================
> > + if(SP_IS_MASK(SP_ITEM(this)->mask_ref->getObject())){
> > + SP_ITEM(this)->mask_ref->getObject()->apply_lpe_to_mask(this);
> > + }
> > + if(SP_IS_CLIPPATH(SP_ITEM(this)->clip_ref->getObject())){
> > + SP_ITEM(this)->clip_ref->getObject()->apply_lpe_to_clippath(this);
> > + }
> > ===================================
> > It can be changed for one or two function/s call/s
> >
> > And for the last, a silly cuestion, Whats better?
> > This:
> > ===================================
> > if(SP_IS_MASK(SP_ITEM(this)->mask_ref->getObject())){
> > SP_ITEM(this)->mask_ref->getObject()->apply_lpe_to_mask(this);
> > }
> > ===================================
> > or this:
> > ===================================
> > SPMask mask = NULL; //is good declare it null first?
> > mask = SP_ITEM(this)->mask_ref->getObject();
> > if(SP_IS_MASK(mask)){
> > mask->apply_lpe_to_mask(this);
> > }
> > ===================================
>
> Best is:
>
> SPMask *mask = SP_ITEM(this)->mask_ref->getObject();
> if(SP_IS_MASK(mask)){
> apply_lpe_to_mask(mask,...);
> }
>
>
> regards,
> Johan
>