Hi Alex,
I've noticed that with addition of DoubleArrowedLine you've also added concept of "inverted" to attach points. This seems really strange from the perspective that all methods startingPointOf: and endingPointOf: are identical only with swapped ifTrue:/ifFalse:
for example ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ startingPointOf: anEdge ^ (inverted ifTrue:[anEdge to] ifFalse:[anEdge from]) position endingPointOf: anEdge ^ (inverted ifTrue: [anEdge from] ifFalse: [anEdge to]) position ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
this is harder to read and more error-prone than ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ startingPointOf: anEdge ^ anEdge from position endingPointOf: anEdge ^ anEdge to position ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ plus there is code duplication. And this is just the simplest attach point.
Since I should also add "inverted" to my attach points that are in Roassal (RTRectangleAttachPoint, ...) maybe the actual computation should be delegated?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ RTAttachPoint>>startingPointOf: anEdge ^ inverted ifTrue: [ self basicEndingPointOf: anEdge ] ifFalse: [ self basicStartingPointOf: anEdge ]
RTAttachPoint>>endingPointOf: anEdge ^ inverted ifTrue: [ self basicStartingPointOf: anEdge ] ifFalse: [ self basicEndingPointOf: anEdge ]
RTAttachPoint>>basicStartingPointOf: anEdge ^ self subclassResponsibility
RTAttachPoint>>basicEndingPointOf: anEdge ^ self subclassResponsibility ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This also will not break current implementation but it will give opportunity to improve them.
And if there was a situation where the implementation would be actually different for inverted (and not just swapped values), than you could still just implement startingPointOf:/endingPointOf:
Thanks, Peter