Skip to content

121 feature implement elementextensions#213

Open
antoineatstariongroup wants to merge 3 commits intodevelopmentfrom
121-feature-implement-elementextensions
Open

121 feature implement elementextensions#213
antoineatstariongroup wants to merge 3 commits intodevelopmentfrom
121-feature-implement-elementextensions

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

Fix #121

Implemented and tested extensions method for Derived Properties and Operations

@antoineatstariongroup antoineatstariongroup linked an issue Apr 3, 2026 that may be closed by this pull request
14 tasks
@antoineatstariongroup antoineatstariongroup marked this pull request as ready for review April 3, 2026 10:09
/// </summary>
/// <param name="name">The name to verifies</param>
/// <returns><c>true</c> if the name is a basic name, <c>false</c> otherwise</returns>
public static bool QueryIsBasicName(this string name)
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.

rename to QueryIsValidBasicName

/// <summary>
/// Provides access to specification defined keywords
/// </summary>
public static class Keywords
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.

can this be generated you think, or are these only in the PDF document?

internal static IElement ComputeOwnedMemberElement(this IOwningMembership owningMembershipSubject)
{
throw new NotSupportedException("Create a GitHub issue when this method is required");
return owningMembershipSubject == null ? throw new ArgumentNullException(nameof(owningMembershipSubject)) : owningMembershipSubject.OwnedRelatedElement.Single();
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.

Single throws an exception if it is not set... is that what you want to happen?

if you do want to throw an exception, i propose we make it an explicit sysml2 related exception basicaly stating the model is incomplete.

.Where(name => name != null)
.ToHashSet(StringComparer.Ordinal);

var importedMembershipsNameFrequency = importedMemberships
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.

Null MemberName handling: GroupBy(x => x.MemberName, ...), if MemberName is null, all null-named memberships will be grouped together under a null key. Then on line 221, ToFrozenDictionary(g =>
g.Key, ...) will try to use null as a dictionary key. FrozenDictionary with StringComparer.Ordinal will throw ArgumentNullException if any membership has a null MemberName. You need to filter out null names before the GroupBy, e.g.:
.Where(x => x.MemberName != null)
.GroupBy(x => x.MemberName, StringComparer.Ordinal)

{
var name = x.MemberName;

return !string.IsNullOrWhiteSpace(name) && !ownedMembershipNames.Contains(name) && (!importedMembershipsNameFrequency.TryGetValue(name, out var frequency) || frequency <= 1);
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 filter !string.IsNullOrWhiteSpace(name) excludes memberships with null/empty names from the result. This means unnamed imported memberships are silently dropped. Is that intentional per the SysML
spec? The spec says to exclude those with "distinguishability collisions" — unnamed memberships arguably don't collide. Worth verifying against the spec.

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 ElementExtensions

2 participants