Clipper.Offset, C# : DoGroupOffset

[philstopford]

Reviewing warnings in Rider, in this method, this line causes me to pause:

    Path64 p = pathIt.Current;

It is possible for this to assign a null to a non-nullable entity, it seems. Later on the code then queries for p.Count and if p is null, bad things would happen.

[AngusJohnson]

Hi Phil, I'm not really sure what you're asking. Here's the immediate context ...

while (pathIt.MoveNext())
{
  Path64 p = pathIt.Current;

So it seems to me that pathIt.Current will always be assigned otherwise pathIt.MoveNext() would return false.

Also, I'm pretty certain that each member of inPaths will be an assigned Path64 object though possibly an empty one.

[philstopford]

Rider indicates that you could have pathIt.Current as null, and then p would be null. This causes a problem initially in that specific line because you'd need something like Path64? p = pathIt.Current; to allow that to be done. If you do that, though, it would cause the subsequent check against p.Count to fail with a null reference exception. This is where I paused. Should pathIt.Current ever be null, what should this function do then? If it can't be null, there probably needs to be a specific indication in the code.

I'm not sure what the right approach is here, because in various other places the code does expect and manage null values for Path instances.

[AngusJohnson]

Rider indicates that you could have pathIt.Current as null, and then p would be null.

But I'm far from convinced that Rider is right in this instance (see my post above).

[philstopford]

If you look at the code for the List that backs your entity (pathIt.Current):

      #nullable enable
      /// <summary>Gets the element at the current position of the enumerator.</summary>
      /// <returns>The element in the <see cref="T:System.Collections.Generic.List`1" /> at the current position of the enumerator.</returns>
      public T Current { get; }

[AngusJohnson]

If you look at the code for the List that backs your entity (pathIt.Current):

But MoveNext() returns false once Current is no longer in range.

[philstopford]

So we can probably help the tool by asserting that .Current is never null with a check or Path64 p = pathIt.Current!;