xmlgraphics-general mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon Pepping <spepp...@leverkruid.eu>
Subject Re: [VOTE] Merge XGC color branch into Trunk
Date Tue, 14 Dec 2010 08:22:20 GMT
On Mon, Dec 13, 2010 at 09:33:17PM +0100, Jeremias Maerki wrote:
> On 13.12.2010 20:43:43 Simon Pepping wrote:
> > On Mon, Dec 13, 2010 at 03:54:44PM +0100, Jeremias Maerki wrote:
> > > I think I have addressed all concerns now. I've switched from using
> > > equals() to ColorUtil.isSameColor(Color, Color). That should take care
> > > of the equals contract problem. If the color branch is now in a state
> > > that everybody is happy with, I'll restart the vote.
> > 
> > Don't you mean the following with the class test:
> > 
> > //Consider same-ness only between colors of the same class (not subclasses)
> > //but consider a ColorWithAlternatives without alternatives to be the same as a
Color.
> > Class<?> cl1 = col1.getClass();
> > if (cl1 == ColorWithAlternatives.class
> >         && !((ColorWithAlternatives)col1).hasAlternativeColors()) {
> >     cl1 = Color.class;
> > }
> > Class<?> cl2 = col2.getClass();
> > if (cl2 == ColorWithAlternatives.class
> >         && !((ColorWithAlternatives)col2).hasAlternativeColors()) {
> >     cl2 = Color.class;
> > }
> > if (cl1 != cl2) {
> >     return false;
> > }
> 
> That's about the same, isn't it?

Not quite. Case: col1 is ColorWithAlternatives and has no
alternatives, col2 is some other subclass of Color. In your
formulation skipClassTest is true, and further tests are applied; in
mine false is returned.

I feel that my formulation does straightforwardly what the comment
says it is doing: a ColorWithAlternatives without alternatives is
considered equivalent to a Color. In your formulation, there is a
complex condition, of which it is hard to see what exactly is tested:

!(col1.getClass() == ColorWithAlternatives.class
      && !((ColorWithAlternatives)col1).hasAlternativeColors())
&&
!(col2.getClass() == ColorWithAlternatives.class
      && !((ColorWithAlternatives)col2).hasAlternativeColors())
&&
(col1.getClass() != col2.getClass())

This kind of hard to understand tests make modifying of the code, when
required, difficult. I am just trying to be helpful to you and to
later developers. Feel free to ignore my suggestion.

> > Even so, subclasses of ColorWithAlternatives may come out
> > unexpectedly: An object of ColorWithAlternatives with no alternative
> > colors, and a Color object will be equal. In contrast, an object of a
> > Subclass of ColorWithAlternatives with no alternative colors, and a
> > Color object will not be equal. Why is that so?
> 
> Because I know that a ColorWithAlternatives without actual alternatives
> will behave exactly the same way as the corresponding Color object. But
> I can't say the same about some subclass of ColorWithAlternatives. Maybe
> such a subclass will add functionality that was not foreseen. In such a
> case, isSameColor() will have to be updated. I guess I should just have
> skipped this test to avoid more discussions, just considering the
> objects of two different classes different. After all, this is just
> about state checking and who cares about the rarest of rare cases where
> two such color objects follow each other?

ColorWithAlternatives is bound by the behaviour of Color.equals.
Similarly, future subclasses of ColorWithAlternatives are bound by the
behaviour of their superclass.

I feel strongly about this one. Unexpected behaviour for future
subclassing is what triggered this discussion.

> > The part where alternative colors are compared, could well be a method
> > of ColorWithAlternatives, putting the logic in the right class.
> 
> Do you feel strongly about that? I don't see the advantage, especially
> since that would add a public method only used by ColorUtil.

No.

These are my suggestions to you. I will not again react to this issue.
Please merge ASAP.

> Jeremias Maerki
> 
> PS: anyone want to write a master thesis on this?

Sorry for being more theoretically minded. I appreciate your numerous
contributions to FOP.

Simon

---------------------------------------------------------------------
To unsubscribe, e-mail: general-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: general-help@xmlgraphics.apache.org


Mime
View raw message