Skip to content
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

Jedd/cxpla 154 enforce nullability analysers in objects and use required #202

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,6 @@ dotnet_diagnostic.ca1508.severity = warning # Avoid dead conditional code
dotnet_diagnostic.ca1509.severity = warning # Invalid entry in code metrics configuration file
dotnet_diagnostic.ca1861.severity = none # Prefer 'static readonly' fields over constant array arguments if the called method is called repeatedly and is not mutating the passed array (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1861)

dotnet_diagnostic.cs8618.severity = suggestion # nullable problem


# Performance rules
Expand Down
9 changes: 2 additions & 7 deletions src/Speckle.Objects/Geometry/Pointcloud.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,11 @@ namespace Speckle.Objects.Geometry;
[SpeckleType("Objects.Geometry.Pointcloud")]
public class Pointcloud : Base, IHasBoundingBox, ITransformable<Pointcloud>
{
/// <summary>
/// Constructs an empty <see cref="Pointcloud"/>
/// </summary>
public Pointcloud() { }

/// <summary>
/// Gets or sets the list of points of this <see cref="Pointcloud"/>, stored as a flat list of coordinates [x1,y1,z1,x2,y2,...]
/// </summary>
[DetachProperty, Chunkable(31250)]
public required List<double> points { get; set; } = new();
public required List<double> points { get; set; }

/// <summary>
/// Gets or sets the list of colors of this <see cref="Pointcloud"/>'s points., stored as ARGB <see cref="int"/>s.
Expand All @@ -38,7 +33,7 @@ public Pointcloud() { }
/// The unit's this <see cref="Pointcloud"/> is in.
/// This should be one of <see cref="Units"/>
/// </summary>
public string units { get; set; }
public required string units { get; set; }

/// <inheritdoc/>
public Box? bbox { get; set; }
Expand Down
83 changes: 41 additions & 42 deletions src/Speckle.Objects/Geometry/Surface.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Diagnostics.CodeAnalysis;
using Speckle.Objects.Other;
using Speckle.Objects.Primitive;
using Speckle.Sdk.Common;
Expand All @@ -11,45 +12,43 @@ namespace Speckle.Objects.Geometry;
[SpeckleType("Objects.Geometry.Surface")]
public class Surface : Base, IHasBoundingBox, IHasArea, ITransformable<Surface>
{
/// <summary>
/// Constructs a new empty <see cref="Surface"/>
/// </summary>
[Obsolete("Constructor should only be used by serializer, use one of the other constructors instead")]
public Surface()
{
applicationId = null;
pointData = new List<double>();
pointData = [];
}

/// <summary>
/// Constructs a new empty <see cref="Surface"/>
/// </summary>
/// <param name="units">The units this surface is modeled in</param>
/// <param name="applicationId">This surface's unique identifier on a specific application</param>
public Surface(string units = Units.Meters, string? applicationId = null)
public Surface(List<List<ControlPoint>> controlPoints)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this constructor? I know the rhino surface converter is currently using it, but isn't it better to pass the control points into pointData instead (and not even convert points in the rhino surface converter)?

{
SetControlPoints(controlPoints);
}

public Surface(IList<double> pointData, int countU, int countV)
{
this.applicationId = applicationId;
this.units = units;
this.pointData = pointData;
this.countU = countU;
this.countV = countV;
}

/// <summary>
/// The degree of the surface in the U direction
/// </summary>
public int degreeU { get; set; }
public required int degreeU { get; set; }

/// <summary>
/// The degree of the surface in the V direction
/// </summary>
public int degreeV { get; set; }
public required int degreeV { get; set; }

/// <summary>
/// Determines if the <see cref="Surface"/> is rational.
/// </summary>
public bool rational { get; set; }
public required bool rational { get; set; }

/// <summary>
/// The raw data of the surface's control points. Use GetControlPoints or SetControlPoints instead of accessing this directly.
/// The raw data of the surface's control points. Use <see cref="GetControlPoints"/> or <see cref="SetControlPoints"/> instead of accessing this directly.
/// </summary>
public List<double> pointData { get; set; }
public IList<double> pointData { get; set; }

/// <summary>
/// The number of control points in the U direction
Expand All @@ -64,38 +63,38 @@ public Surface(string units = Units.Meters, string? applicationId = null)
/// <summary>
/// The knot vector in the U direction
/// </summary>
public List<double> knotsU { get; set; }
public required List<double> knotsU { get; set; }

/// <summary>
/// The knot vector in the V direction
/// </summary>
public List<double> knotsV { get; set; }
public required List<double> knotsV { get; set; }

/// <summary>
/// The surface's domain in the U direction
/// </summary>
public Interval domainU { get; set; }
public required Interval domainU { get; set; }

/// <summary>
/// The surface's domain in the V direction
/// </summary>
public Interval domainV { get; set; }
public required Interval domainV { get; set; }

/// <summary>
/// Determines if a surface is closed around the <see cref="domainU"/>.
/// </summary>
public bool closedU { get; set; }
public required bool closedU { get; set; }

/// <summary>
/// Determines if a surface is closed around the <see cref="domainV"/>
/// </summary>
public bool closedV { get; set; }
public required bool closedV { get; set; }

/// <summary>
/// The unit's this <see cref="Surface"/> is in.
/// This should be one of <see cref="Units"/>
/// </summary>
public string units { get; set; }
public required string units { get; set; }

/// <inheritdoc/>
public double area { get; set; }
Expand All @@ -116,7 +115,7 @@ public bool TransformTo(Transform transform, out Surface transformed)
}
}

transformed = new Surface
transformed = new Surface(ptMatrix)
{
degreeU = degreeU,
degreeV = degreeV,
Expand All @@ -131,7 +130,6 @@ public bool TransformTo(Transform transform, out Surface transformed)
knotsV = knotsV,
units = units,
};
transformed.SetControlPoints(ptMatrix);

return true;
}
Expand Down Expand Up @@ -172,6 +170,9 @@ public List<List<ControlPoint>> GetControlPoints()
/// </summary>
/// <param name="value">A 2-dimensional array of <see cref="ControlPoint"/> instances.</param>
/// <remarks>The <paramref name="value"/> must be ordered following directions "[u][v]"</remarks>
[MemberNotNull(nameof(pointData))]
[MemberNotNull(nameof(countU))]
[MemberNotNull(nameof(countV))]
public void SetControlPoints(List<List<ControlPoint>> value)
{
List<double> data = new();
Expand Down Expand Up @@ -229,29 +230,27 @@ public List<double> ToList()
/// <returns>A new <see cref="Surface"/> with the provided values.</returns>
public static Surface FromList(List<double> list)
{
var srf = new Surface
var pointCount = (int)list[11];
var knotsUCount = (int)list[12];
var knotsVCount = (int)list[13];
var countU = (int)list[2];
var countV = (int)list[3];

var pointData = list.GetRange(14, pointCount);
var u = list[^1];

return new Surface(pointData, countU, countV)
{
degreeU = (int)list[0],
degreeV = (int)list[1],
countU = (int)list[2],
countV = (int)list[3],
rational = list[4] == 1,
closedU = list[5] == 1,
closedV = list[6] == 1,
domainU = new Interval { start = list[7], end = list[8] },
domainV = new Interval { start = list[9], end = list[10] },
knotsU = list.GetRange(14 + pointCount, knotsUCount),
knotsV = list.GetRange(14 + pointCount + knotsUCount, knotsVCount),
units = Units.GetUnitFromEncoding(u),
};

var pointCount = (int)list[11];
var knotsUCount = (int)list[12];
var knotsVCount = (int)list[13];

srf.pointData = list.GetRange(14, pointCount);
srf.knotsU = list.GetRange(14 + pointCount, knotsUCount);
srf.knotsV = list.GetRange(14 + pointCount + knotsUCount, knotsVCount);

var u = list[^1];
srf.units = Units.GetUnitFromEncoding(u);
return srf;
}
}
1 change: 1 addition & 0 deletions src/Speckle.Sdk/Speckle.Sdk.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<PackageId>Speckle.Sdk</PackageId>
<Description>The .NET SDK for Speckle</Description>
<PackageTags>$(PackageTags) core sdk</PackageTags>
<NoWarn>$(NoWarn);CS8618</NoWarn>
</PropertyGroup>

<PropertyGroup Label="Nuget Package Properties">
Expand Down
Loading