Conversation
MaxStachon
requested review from
luepges and
schmalzing
and removed request for
schmalzing
|
✔️ Changes pass the MontiVerse
|
| /** | ||
| * @param node class to check. | ||
| */ | ||
| /** @param node class to check. */ |
There was a problem hiding this comment.
not really a javadoc - -> remove?
| import java.util.*; | ||
|
|
||
| /** | ||
| * Checks that there are not multiple occurrences of the same association between types |
There was a problem hiding this comment.
give an example?
What does "occurrences of the same association " mean?
| * helper-method to find types by full-name | ||
| */ | ||
|
|
||
| private static boolean isNavigable(ASTCDAssociation assoc1, AssocSide side1) { |
There was a problem hiding this comment.
Why static?
=> removes chance for overriding this method
| AssocSide.LEFT))) { | ||
| checkRef(node, findTypeByFullName(assoc1, assoc1.getRightQualifiedName().getQName()), | ||
| findTypeByFullName(assoc2, assoc2.getRightQualifiedName().getQName()), assoc1); | ||
| if (deriveRoleName(assoc1, AssocSide.LEFT).equals(deriveRoleName(assoc2, AssocSide.LEFT)) |
There was a problem hiding this comment.
why equals with enum (instead of ==)
| } | ||
|
|
||
| Log.error("0xCDCE2: Could not find: " + fullName + "."); | ||
| Log.error("0xCDCE2: Could not find: " + fullName + ".", node.get_SourcePositionStart()); |
There was a problem hiding this comment.
In what context could fullName not be found?
|
|
||
| Log.error("0xCDCE2: Could not find: " + fullName + "."); | ||
| Log.error("0xCDCE2: Could not find: " + fullName + ".", node.get_SourcePositionStart()); | ||
| return null; |
There was a problem hiding this comment.
Does a junit Test exist for this error?
Cause CoCos aggregate errors (quickFail==false), but findTypeByFullName might return null -> NPE
| type2.getName()), assoc1.isPresent_SourcePositionStart() ? assoc1 | ||
| .get_SourcePositionStart() : null, assoc1.isPresent_SourcePositionEnd() ? assoc1 | ||
| .get_SourcePositionEnd() : null); | ||
| Log.error(String.format("0xCDCE1: %s has duplicate associations at %s and %s.", type1 |
There was a problem hiding this comment.
What does duplicate associations mean?
And also report the name of the assoc (if given?)
There was a problem hiding this comment.
And add some text, that duplicate associations are not supported
| if (nextType.getFullName().equals(type2.getSymbol().getFullName())) { | ||
| Log.error(String.format("0xCDCE6: %s redefines an association of %s.", type1.getName(), | ||
| type2.getName())); | ||
| Log.error(String.format("0xCDCE6: %s redefines an association of %s from %s at %s", type1 |
There was a problem hiding this comment.
Add some text, that "redefines an association " are not supported.
| public void testNonOpposedAssocs3() throws IOException { | ||
| String model = "classdiagram DuplicateAssocs {" + " class A; class B;" | ||
| + " association A -> B;" + " association B <- A;" + "}"; | ||
| runTestForErrorCode(model, ERROR_CODE); |
There was a problem hiding this comment.
would it make sense to also check for the content of the error message?
Would help making sure the error messages are useful to users of a tool / makes review easier
2 participants
CDUniqueAssociationnow allows "duplicate" associations with explicitly opposing navigation directions: e.g., A -> B, A <- B.CDUniqueAssociationandCDUniqueAssociationInHierarchyoutput SourcePosition of the associations