121 feature implement elementextensions#213
121 feature implement elementextensions#213antoineatstariongroup merged 8 commits intodevelopmentfrom
Conversation
| /// </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) |
There was a problem hiding this comment.
rename to QueryIsValidBasicName
| /// <summary> | ||
| /// Provides access to specification defined keywords | ||
| /// </summary> | ||
| public static class Keywords |
There was a problem hiding this comment.
can this be generated you think, or are these only in the PDF document?
There was a problem hiding this comment.
It is part of rules, can be generated at the same time than the Textual notation
| 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(); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
The multiplicity of this property is [1..1] so Single is correct. I can create a specific Exception if you want
There was a problem hiding this comment.
please create a dedicated exception
| .Where(name => name != null) | ||
| .ToHashSet(StringComparer.Ordinal); | ||
|
|
||
| var importedMembershipsNameFrequency = importedMemberships |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
…wer about PR's comments
| 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(); |
There was a problem hiding this comment.
please create a dedicated exception
| /// Provides equality comparison for string, handling null case | ||
| /// </summary> | ||
| public sealed class NullSafeStringComparer : IEqualityComparer<string> | ||
| { |
There was a problem hiding this comment.
please check the warnings of Sonar... seems there is an issue with the docs
| /// <summary> | ||
| /// The <see cref="SysMl2ModelException"/> provide custom exception that should be used when a SysML 2 rule violation appears into a model | ||
| /// </summary> | ||
| public class SysMl2ModelException: Exception |
There was a problem hiding this comment.
please update docs and rename to ModelException if that is the intent of course
| using System; | ||
|
|
||
| /// <summary> | ||
| /// The <see cref="ModelException"/> provide custom exception that should be used when a SysML 2 is incomplete and is not compliant with defined multiplicity |
There was a problem hiding this comment.
docs need an update -> IncompleteModelException
| @@ -179,7 +205,7 @@ internal static string ComputeQualifiedName(this IElement elementSubject) | |||
| [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] | |||
There was a problem hiding this comment.
remove this annotation
| @@ -60,7 +60,7 @@ internal static string ComputeMemberElementId(this IMembership membershipSubject | |||
| [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] | |||
| internal static INamespace ComputeMembershipOwningNamespace(this IMembership membershipSubject) | |||
There was a problem hiding this comment.
remove this annotation
| @@ -45,7 +48,7 @@ internal static class NamespaceExtensions | |||
| [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] | |||
There was a problem hiding this comment.
remove this annotation
| @@ -60,7 +63,7 @@ internal static List<IMembership> ComputeImportedMembership(this INamespace name | |||
| [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] | |||
There was a problem hiding this comment.
remove this annotation
| @@ -75,7 +78,7 @@ internal static List<IElement> ComputeMember(this INamespace namespaceSubject) | |||
| [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] | |||
There was a problem hiding this comment.
remove this annotation
antoineatstariongroup
deleted the
121-feature-implement-elementextensions
branch
2 participants
Prerequisites
Description
Fix #121
Implemented and tested extensions method for Derived Properties and Operations