AngusJohnson/Clipper2

C#, Clipper.FileIO : null comparisons always false in many cases

philstopford opened this issue · 2 comments

I'm not sure what the expectation is here from the origin side, but there are a bunch of null checks here that seem to be problematic because the left side makes the comparison always false.

For example, these checks are always false (just a sampling - there are more indicated through this class):

    public static Paths64 PathFromStr(string s)
    {
      if (s == null) return new Paths64();
    public static bool LoadTestNum(string filename, int num,
      Paths64 subj, Paths64 subj_open, Paths64 clip,
      out ClipType ct, out FillRule fillRule, out long area, out int count, out string caption)
    {
      if (subj == null) subj = new Paths64(); else subj.Clear();
      if (subj_open == null) subj_open = new Paths64(); else subj_open.Clear();
      if (clip == null) clip = new Paths64(); else clip.Clear();

This one gives me pause because PathFromStr() always returns a Paths64 value, so the null check would appear to be meaningless. I've not touched this in my associated work posted in the PR because the inconsistency here makes me wonder which side should be corrected.

          Paths64 paths = PathFromStr(s); //0 or 1 path
          if (paths == null || paths.Count == 0)

Somewhat related, this same null related approach needs a change here:

        string s = reader.ReadLine();
        if (s == null) break;

perhaps to something like this:

        string? s = reader.ReadLine();
        if (s == null) break;

PR makes the parameters to the methods support nulls being passed in; speculative, not sure if this is correct or the null checks are superfluous......

Thanks again Phil.
I haven't closely checked your changes in your PR but I'm confident that they are all for the better.