- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.3k
32943-Sugar for HierarchyId path generation #33062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
30bbe57
              af7a76c
              8dc98c7
              e74ea6d
              595a85e
              44a2808
              c1e2925
              d926406
              b214f1c
              a6c9ae9
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -2,6 +2,7 @@ | |
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|  | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Text; | ||
| using System.Text.Json.Serialization; | ||
| using Microsoft.EntityFrameworkCore.Internal; | ||
| using Microsoft.SqlServer.Types; | ||
|  | @@ -25,14 +26,24 @@ public HierarchyId() | |
| } | ||
|  | ||
| /// <summary> | ||
| /// Initializes a new instance of the<see cref="HierarchyId" /> class. Equivalent to <see cref="Parse" />. | ||
| /// Initializes a new instance of the<see cref="HierarchyId" /> class. Equivalent to <see cref="Parse(string?)" />. | ||
| /// </summary> | ||
| /// <param name="value">The string representation of the node.</param> | ||
| public HierarchyId(string value) | ||
| : this(SqlHierarchyId.Parse(value)) | ||
| { | ||
| } | ||
|  | ||
| /// <summary> | ||
| /// Initializes a new instance of the<see cref="HierarchyId" /> class. Equivalent to <see cref="Parse(HierarchyId, IReadOnlyList{int})" />. | ||
| /// </summary> | ||
| /// <param name="parentHierarchyId">The parent HierarchyId of node.</param> | ||
| /// <param name="parentId">The parent Id of current node. It can be more than one element if want have path like: "/1/2/3.1/", otherwise one element for have path like: "/1/2/3/".</param> | ||
| public HierarchyId(HierarchyId parentHierarchyId, IReadOnlyList<int> parentId) | ||
| : this(Parse(parentHierarchyId,parentId)) | ||
| { | ||
| } | ||
|  | ||
| /// <summary> | ||
| /// Initializes a new instance of the<see cref="HierarchyId" /> class. | ||
| /// </summary> | ||
|  | @@ -63,6 +74,33 @@ public static HierarchyId GetRoot() | |
| public static HierarchyId? Parse(string? input) | ||
| => (HierarchyId?)SqlHierarchyId.Parse(input); | ||
|  | ||
| /// <summary> | ||
| /// Converts the <paramref name= "parentHierarchyId" /> and <paramref name= "parentId" /> of a node to a <see cref="HierarchyId" /> value. | ||
| /// </summary> | ||
| /// <param name="parentHierarchyId">The parent HierarchyId of node.</param> | ||
| /// <param name="parentId">The parent Id of current node. It can be more than one element if want have path like: "/1/2/3.1/", otherwise one element for have path like: "/1/2/3/".</param> | ||
| /// <returns>A <see cref="HierarchyId" /> value.</returns> | ||
| [return: NotNullIfNotNull(nameof(parentHierarchyId))] | ||
| [return: NotNullIfNotNull(nameof(parentId))] | ||
| public static HierarchyId? Parse(HierarchyId parentHierarchyId , IReadOnlyList<int> parentId) | ||
|          | ||
| => GenerateHierarchyIdBasedOnParent(parentHierarchyId, parentId); | ||
|  | ||
| //This Method can move to "SqlHierarchyId in Microsoft.SqlServer.Types", if we don't want put it in this abstraction. | ||
| private static HierarchyId GenerateHierarchyIdBasedOnParent(HierarchyId parent, IReadOnlyList<int> parentId) | ||
| { | ||
| if (parent is null) | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add curlies here. In general, try to follow the formatting of other cs files in the project. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved in This Commit. | ||
| return HierarchyId.GetRoot(); | ||
|  | ||
| if (parentId.Count < 1) | ||
| return parent; | ||
|  | ||
| var specificPath = new StringBuilder(parent.ToString()); | ||
| specificPath.Append(string.Join(".", parentId)); | ||
| specificPath.Append('/'); | ||
|  | ||
| return HierarchyId.Parse(specificPath.ToString()); | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this can ever return null, right? In which case the return type here should be non-nullable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you mean this method will never have a null output, it is correct. I Modified the new  Also, the main overload of the      [return: NotNullIfNotNull(nameof(input))]
    public static HierarchyId? Parse(string? input)
        => (HierarchyId?)SqlHierarchyId.Parse(input); | ||
| } | ||
|  | ||
| /// <inheritdoc /> | ||
| public virtual int CompareTo(HierarchyId? other) | ||
| => _value.CompareTo((SqlHierarchyId)other); | ||
|  | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a need for both a
Parsemethod and a constructor?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it because another overload of the
ParseMethod is used in theconstructor, and I think it's not a bad idea to include it. It can be removed and I removed theconstructorwith this overload in This Commit