Path.js code comment about inkjs

[y-lohse]

Right, it most likely doesn't, except when the two pathes are literally the same, but that's a bit limited. Thanks for reminding me.

Though the question whether it uses == or the IEquatable interface is a good one. @joethephish do you mind sharing some C# insight on this?

[joethephish]

I found the C# docs super confusing on this issue....!

When searching the current version of the codebase, that comment seems to no longer exist in my C# implementation, I seem to have swapped it back to use SequenceEqual, with the assumption being that since Path.Component implements IEquatable, it'll "do the right thing".

Given that we don't seem to have had any related bugs, I'm assuming it is doing the right thing! (yeah, I'm a professional developer, honest)

So yeah, if the JS implementation uses ==, it'll need to iterate and do a proper equality comparison.

[Glidias]

so, should the IEquatable's Equals() impl be used instead (if available) when checking for equality for JS/other targets during SequenceEqual(), or would plain "==" always suffice?

[y-lohse]

When searching the current version of the codebase, that comment seems to no longer exist in my C# implementation,

I'm pretty sure I wrote that comment as a note to myself and forgot about it later, so it never was in the C# version.

so, should the IEquatable's Equals() impl be used instead (if available) when checking for equality for JS/other targets during SequenceEqual(), or would plain "==" always suffice?

My guess is that it should use the Equals() (which is already implemented anyway). Two pathes should be considered the same if all their components are equal, not only if they are the same instance? And given that object comparisons in javascript aren't the most reliable anyway, it's probably safer.

[Glidias]

Hmm..yea, there are only 2 classes (Path and Component, which implements IEquatable Equals(other:T):Bool , overriding the default Equals<Runtime.Object> implementation used by the Runtime.Object class it extends from. Looking at the code, as you said, the override is probably there for a purpose, and so I've modified my array sequence equality method with the IEquatable type constraint as well.)

public static function arraySequenceEquals<T:IEquatable<T>>(arr1:Array<T>, arr2:Array<T>):Bool {
        if (arr1.length != arr2.length) return false;
        for (i in 0...arr1.length) {
            if (!arr1[i].Equals(arr2[i])) {  // enforced IEquatable constraint
                return false;
            }
        }
        return true;
    }    

For anything outside of that SequenceEquals() case, i can't remember exactly whether there were other cases in the code where Path and Component was checked for "==" and "!=" respectively, though. May probably do a (manual) audit search for such operators being made between/ Path to Path /and /Component to Component/ just in case.

[y-lohse]

Alright, I'll do something similar, though I'll probably inline the function. Can you let me know if you find other instances of == and != in your audit?

[Glidias]

Haven't really looked into yet, but I know for one case... the dev-only debugging method BuildStringOfHierarchy might need to consider it in a sitauation where Path==Path/Component==Component comparisons are made( again...i'm only guessing), and it's more on a yagni-atm basis , since the method isn't actually used in the actual runtime for end-user.

[y-lohse]

Fixed in the next release.