Skip to content

Implemetation NamespaceExtensions#217

Merged
antoineatstariongroup merged 5 commits intodevelopmentfrom
215-feature-implement-namespaceextensions
Apr 17, 2026
Merged

Implemetation NamespaceExtensions#217
antoineatstariongroup merged 5 commits intodevelopmentfrom
215-feature-implement-namespaceextensions

Conversation

@antoineatstariongroup
Copy link
Copy Markdown
Collaborator

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the SysML2.NET code style guidelines
  • I have provided test coverage for my change (where applicable)

Description

  • Update to latest uml4net to fix operation implementation
  • implementation of the Namespace derived properties and Operation

@antoineatstariongroup antoineatstariongroup linked an issue Apr 17, 2026 that may be closed by this pull request
22 tasks
@antoineatstariongroup antoineatstariongroup marked this pull request as ready for review April 17, 2026 11:16
Comment thread SysML2.NET/Extend/NamespaceExtensions.cs
Comment thread SysML2.NET.Tests/Extend/OwningMembershipExtensionsTestFixture.cs
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@antoineatstariongroup antoineatstariongroup merged commit a900fc0 into development Apr 17, 2026
7 checks passed
@antoineatstariongroup antoineatstariongroup deleted the 215-feature-implement-namespaceextensions branch April 17, 2026 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Implement NamespaceExtensions

2 participants