-
15. Re: Precedence
"alesj" wrote:
I'll revert the changes and add name compare on the one's that are not 'comparable'.
If I introduce the name compare - _after_ the transitive closure build - the actual DeployerFlowUnitTestCase.testIntermediateIsRelativelySorted test creates a cycle. :-)
A is after C on inputs/outputs.
B is after A lexicographically,
C is after B lexicographically.
;-) -
16. Re: Precedence
"alesj" wrote:
If I introduce the name compare - _after_ the transitive closure build - the actual DeployerFlowUnitTestCase.testIntermediateIsRelativelySorted test creates a cycle. :-)
A is after C on inputs/outputs.
B is after A lexicographically,
C is after B lexicographically.
;-)
I changed the B into E, and left the other parts of the test unchanged.
OK? :-)
I added name compare + additional cycle check on the newly added name compare. -
17. Re: Precedence
"alesj" wrote:
I changed the B into E, and left the other parts of the test unchanged.
OK? :-)
No that misses the whole point of the test.
A) inputs=test1 outputs=null
B) inputs=null outputs=null
C) inputs=null outputs=test1
It is actually irrelevant where (B) is compared to (A) or (C)
If it just used Ordered.COMPARATOR in the order above then (B) would
get in the way of the sorting.
What is relevant is that A inputs from C, so the correct orders is either
1) C, A, B
2) B, C, A
3) C, B, A
(3) is obviously wrong since C and B are unrelated by attachment
they should be ordered by Ordered.COMPARATOR
but either (1) or (2) is correct.
The test assumes the following heuristic (which was true of the old sort):
"Favour the order in which deployers are deployed".
i.e. Since B was deployed after A and there is no reason why B should be
sorted in front of A, therefore it always remains after A.
i.e. (1) is the correct order. -
18. Re: Precedence
"adrian@jboss.org" wrote:
"alesj" wrote:
I changed the B into E, and left the other parts of the test unchanged.
OK? :-)
No that misses the whole point of the test.
It misses the whole point of the test because in the real case
A=SecurityDeployer (inputs from the EJBRegistrationDeployer's outputs)
B=EJBDeployer (unrelated)
C=EJBRegistrationDeployer
(well actually there are package names as well to consider, but you get the idea :-)
i.e. lexically B is before C. -
19. Re: Precedence
Uf, currently I don't see how to incorporate this into existing algorithm - doing name compare selectively.
-
20. Re: Precedence
That's why I used a bubble sort, since it favours not changing the order unless there is a reason. If it was a simple sort then I'd have just used a comparator with a sorted list. :-)
-
21. Re: Precedence
If your sort has a different heuristic, then that is ok, as long as the order is
predicatable and documented. :-) -
22. Re: Precedence
"adrian@jboss.org" wrote:
What is relevant is that A inputs from C, so the correct orders is either
1) C, A, B
2) B, C, A
3) C, B, A
(3) is obviously wrong since C and B are unrelated by attachment
they should be ordered by Ordered.COMPARATOR
but either (1) or (2) is correct.
The test assumes the following heuristic (which was true of the old sort):
"Favour the order in which deployers are deployed".
i.e. Since B was deployed after A and there is no reason why B should be
sorted in front of A, therefore it always remains after A.
i.e. (1) is the correct order.
Why would (3) be wrong?
Do we really care if there is an unrelated one in the middle?
I'll see if I can document my heuristic. :-) -
23. Re: Precedence
I managed to get tests working with current sort - by not changing the test, but sort impl. ;-)
I still need to document the heuristics - hopefully there is one :-) - and add some more tests accordingly.