Implemetation NamespaceExtensions#217
Conversation
There was a problem hiding this comment.
The SysML v2 rule for importedMemberships excludes members that clash with any
owned member's name or shortName. This implementation only hashes
MemberName. Two imported members with the same MemberShortName (or an owned
member sharing a MemberShortName with an imported member) will slip through.
Either:
- Extend the hash sets to also consider MemberShortName, or
| throw new ArgumentNullException(nameof(mem)); | ||
| } | ||
|
|
||
| var import = namespaceSubject.ownedImport.FirstOrDefault(x => x.ImportedMemberships([]).Contains(mem)); |
There was a problem hiding this comment.
ImportedMemberships([]) is re-evaluated for every ownedImport until a match
is found — O(imports × import cost) per call, and VisibilityOfOperation itself
is called inside ComputeVisibleMembershipsOperation's .Where filter, so the
outer loop multiplies it again. Materialise once:
var import = namespaceSubject.ownedImport.FirstOrDefault(x =>
((ICollection)x.ImportedMemberships([])).Contains(mem));
…and ideally cache across the enclosing ComputeVisibleMembershipsOperation
call by using namespaceSubject.ImportedMemberships([]) once and reusing it.
There was a problem hiding this comment.
CLAUDE.md calls out "prefer indexer syntax over LINQ methods when
applicable". After the count check, OwnedRelatedElement[0] is correct and
faster (avoids the second enumeration that Single() performs).
There was a problem hiding this comment.
namespaceSubject.importedMembership is itself the derived property that our
own ComputeImportedMembership produces. Calling
ownedMembership.Union(importedMembership) plus ComputeImportedMembership(…)
elsewhere ends up recomputing the import chain at least twice per
ComputeMembership call. Consider referencing a single source of truth — e.g.
namespaceSubject.ImportedMemberships([]) — or adding a short // Why: comment
if the duplication is intentional for layering reasons.
Prerequisites
Description