[css-typed-om] "Add two types" algorithm seems overly complicated?

[AtkinsSJ]

Hi, I'm just starting to implement CSSNumericValue and this algorithm seems to be described in a way that is a lot more complicated and verbose than it needs to be.

Step 3 of "add two types" (which currently is displaying incorrectly as step 6) says:

If all the entries of type1 with non-zero values are contained in type2 with the same value, and vice-versa Copy all of type1’s entries to finalType, and then copy all of type2’s entries to finalType that finalType doesn’t already contain. Set finalType’s percent hint to type1’s percent hint. Return finalType.

I'm struggling a bit to understand this. The "if" sounds like it checks all the "BaseType -> exponent" values are the same in type1 and type2. But if that's the case, then we don't need to copy values from both type1 and type2 into finalType, because there can't be anything in type2 that isn't in type1. In fact, finalType just becomes a copy of type1 (which is already a copy of the original in step 1) so we could return type1 directly.

So it should be equivalent to:

If all the entries of type1 with non-zero values are contained in type2 with the same value, and vice-versa Return type1.

Later in that same step, we do the same "compare type1 and type2 and then copy and return" procedure, which could likewise be "Return type1". Then, we're no longer using finalType at all, and can remove it from step 1.

There may be nuances here that I'm missing, so do correct me if I'm wrong! (It wouldn't be the first time. :sweat_smile:)

[tabatkins]

The "if" sounds like it checks all the "BaseType -> exponent" values are the same in type1 and type2.

The if checks that the non-zero entries of type1 and type2 are identical; the final type contains all the entries, including zero-valued entries from either input type.

Zero-valued entries are important to maintain because they indicate that the type is still necessary for the input even if it doesn't show up in the output. calc(20deg * (1px / 1em)) has the type {angle: 1, length: 0}, and can't, for example, be represented as a CSSUnitValue (until it's simplified as a computed or used value, at which point it will be just {angle: 1}). A few of the methods care about this, like .to().

But it's perfectly valid to add a calc(20deg * (1px / 1em)) to a calc(30deg); they have compatible types for this purpose (they'll both resolve to angles, eventually). The result will again have the type {angle: 1, length: 0}, preserving the complexity of the inputs.

[AtkinsSJ]

Ahhhh, thank you very much for the explanation Tab!