Feedback on IsDefault and Default members

[YohDeadfall]

There was PR https://github.com/AvaloniaUI/Avalonia/pull/9590 changing member names IsDefault and Default instead of IsEmpty and Empty. While old names using Empty can be used for area types like Rect they were much better than the current ones for a few reasons:

• There's default keyword not requiring any methods to be added, i.e. use == default or is default.
• New names provide no sematic meaning. What is Default for the current case?

So I suggest to go back to Empty/IsEmpty or use Zero/IsZero instead. The last one is applicable to any numeric type, including vectors and in addition to that it's a way forward to generic math even if it's impossible to implement INumberBase<TSelf> for some types like Rect and Point (vector types can support generic math, by the way).

[MarchingCube]

Yeah, I agree as well. When refactoring our codebase each change to IsDefault felt like we were bleeding semantics. Code that previously could be pretty much read as a sentence now lost all meaning. Also usage got awkward in generic methods since now you need to write default(Rect) instead of Rect.Empty.

if (bounds.IsEmpty) { } vs if (bounds.IsDefault) is one example of lost semantics.

[grokys]

I've also noticed this when upgrading. It's a small thing but feels like (negative) change for the sake of change.

pinging @robloo

[pr8x]

So I suggest to go back to Empty/IsEmpty or use Zero/IsZero instead.

IMO we could have both. IsEmpty checks for zero area and IsZero checks if the Rect is (0,0,0,0).

[robloo]

First, the code should still compile, old members were obsolete not removed. That should help as we discuss this again.

However, has everyone read the original issue and reasoning there? Ideally, this discussion would be continued over there for full history.

https://github.com/AvaloniaUI/Avalonia/issues/7762#issuecomment-1295897659

This change was an attempt to unify the API as various types are not consistent with these members and were also not consistent with WPF. I made a table showing this in more detail. I want to be sure everyone is looking at the big picture here before we decide to undo some things.

I am aware of some cases the semantics actually improved by the IsDefault change and then there is the case above and by @maxkatz in #7762 where there seems to be a loss of semantics. So it may not be a one size fits all fix.

Empty doesn't work for most types I think though, it is also ambiguous yet we have gotten used to it. It works best for collections or things that hold content. An empty border thickness isn't exactly grammatically correct. Default border thinking makes more sense but perhaps needs to convey zero somehow which Empty did imply.

Anyway, I like the idea of adding Zero/IsZero to numeric types and I think that makes sense. Then keeping Default/IsDefault as-is for everything as this also makes sense in some cases. Certain types initialize with non-zero values to essentially indicate they are uninitialized and not zero (Rect in WPF and the old Color structs in drawing are examples).

[robloo]

it's a way forward to generic math

That's a very good point and should be factored in. Vector at least needs to take this into account and use Zero/IsZero IMO.

Also usage got awkward in generic methods since now you need to write default(Rect) instead of Rect.Empty

That is perfectly clear to read IMO. The issues can happen if you just write 'default'.

[robloo]

I'm going to put a new table together when I have time for feedback here. I think most of the issues can be fixed with Zero/IsZero additions in some places. Zero cornerradius, Zero thickness make more sense as well. The argument of alignment with numerics is a strong one.

IsDefault should stay around for usage by the types that initialize with non-zero values to communicate the information that they are default and different than Zero. It also wont make a new instance for comparison like '== default' or 'is default' would. As mentioned above some types use this in the past (not Avalonia so far) and it also aligns nicely with the default keyword in C# (which is why the Defailt/Empty field can be removed as well, it's redundant).

Rect is probably the most interesting case.

1. Empty for area does make some sense for Rect; however, I suppose when you get into negatives this becomes a little ambiguous again. Also, when there is a width but zero height (and vice versa), it doesn't have area but also shouldn't really be drawn. So code needs to probably handle this by checking if width or height are zero rather than using IsZero or a resurrected IsEmpty property. The old IsEmpty property wasn't so specific and was actually just checking for zero which is probably incorrect for a type like this that has two dimensions and setting any to zero, reguardless of the other, will collapse it.
2. IsZero brings back the clarity of IsEmpty in the cases mentioned. It makes it perfectly clear what this does as well removing all ambiguity IMO. This will only be true when all components are zero.
3. IsDefault could actually be different that IsZero (old IsEmpty) https://github.com/AvaloniaUI/Avalonia/issues/7762#issuecomment-1406052217. We might need to align with WPF at some point for this. So probably keeping this member also makes sense here.

[robloo]

Below is a new proposal using Zero/IsZero in place of Empty/IsEmpty in most places. Please everyone with an interest provide your feedback.

• It might be a good idea to keep IsDefault for most types instead of obsoleting it. This is especially true for Rect which may have IsDefault be a different meaning than IsZero in the future. Rect would need to add this property in the below graphic.
• Again, I'm keeping the old Empty/IsEmpty members and only marking them obsolete for 11.0
• Zero/IsZero actually makes sense for the vast majority of the types. This was a great suggestion and does match well with INumberBase (https://learn.microsoft.com/en-us/dotnet/api/system.numerics.inumberbase-1.iszero?view=net-7.0) with the exception of IsZero being a property instead of a method. BigInteger has an IsZero property though so there is some convention for this.
• BoxShadow is another example where Empty/Zero meanings both aren't ideal. In that case its better to use default so I plan to add it.
• Vector types should have been using a Zero member all along https://learn.microsoft.com/en-us/dotnet/api/system.numerics.vector3.zero?view=net-7.0.

Here is the members that types had before any changes for 11.0. Note again the inconsistency even with WPF.

[pr8x]

@robloo So only BoxShadow should use IsDefault now? TBH for the sake of consistency I would have them all use IsZero and call it a day.

It might be a good idea to keep IsDefault for most types instead of obsoleting it. This is especially true for Rect which may have IsDefault be a different meaning than IsZero in the future. Rect would need to add this property in the below graphic.

I would disagree. Having two properties that do the same thing is confusing and I honestly don't think we need an IsDefault with different semantics.

[YohDeadfall]

The same, I see no value in IsDefault. It just bloats the public API and adds zero value

[robloo]

Certain types in WPF, such as Rect and Color, initialize with negative or special non-zero state values if they aren't built through the constructor. These negative/non-zero values represent a "default" unset state. This is clearly different than zero.

BoxShadow isn't a numerical type so having Zero/IsZero makes no sense. IsDefault makes perfect sense here. Keep in mind Avalonia itself is already using IsDefault for 4 types and that is the modern convention instead of IsEmpty. This was all discussed in #7762.

So I have to disagree with you in both of those points.

I do still plan on adding IsDefault to color as well as it seems to be needed for some ColorPicker features.

[YohDeadfall]

WPF is a different framework with its own implementation which may be different from Avalonia. So, if you look at Rect which you mentioned, you find that a default value is a zero initialized structure.

[pr8x]

initialize with negative or special non-zero state values if they aren't built through the constructor

IMO This seems like a horrible anti-pattern that we should certainly not adopt from WPF. Rect and color should plain data types without any magic, special states or anything.

needed for some ColorPicker features.

How/why does ColorPicker rely on a IsDefault property?

The problem I have with IsDefault is mostly that it's not really descriptive. What default does it mean? The C# default default or some other default? If I see Rect.IsDefault somewhere I always need to check what it does. Rect.IsEmpty or Rect.IsZero on the other hand clearly describe what they're doing.

[robloo]

How/why does ColorPicker rely on a IsDefault property?

It doesn't yet. However, the default color in some cases is fully transparent. This is fine except for the cases where the user selects the color (or two components of it) from the spectrum for the first time. If it's the first user selection it is implied that transparency and the 3rd component should actually be max value instead of zero. Therefore, logic is needed to detect this case and then automatically set transparency/3rd component. The problem is that should only be done the first time a user makes a selection and all other time values left unchanged. I need some way of communicating user has NOT set the value yet. Using nullability is an ugly tear up and it's much easier to just check if the color has a default initialized state instead of a valid color. This was done in older frameworks. This issue must be fixed for usability of the color picker and I also no NOT want to make it nullable. So unless I can get creative with logic inside the color picker itself (and I have a few ideas left) just using a default (not zero) color is by far the best solution. Conceptually, or in terms of information, this is the same as double.NaN and its unarguable how useful that is in the framework and higher level algorithms.

So again I disagree this is an anti-pattern and should be avoided at all costs. In fact NumberBox really could have used decimal.NaN but it wasnt available. Double.NaN is extremely useful. IsDefault for some types is the same concept.

For clearly numeric types I agree IsZero is the best. Its possible to need both IsZero and IsDefault though in terms of information. There are only 3 types I think we are still debating though so we are getting closer to agreement.

[robloo]

So, if you look at Rect which you mentioned, you find that a default value is a zero initialized structure.

Yes Avalonia is different and has been able to keep Rect simpler so far. There are several cases where to get the last features it becomes obvious why WPF did something though. There is no guarantee that in the future we wouldn't have to align more with WPF to be able to match full functionality. In fact there are many cases where Avalonia's simplicity is a direct result of skipping more complex WPF features. If you want to support those features, code needs to be closer to WPF. There are several cases where Avalonia has had to backtrack and start doing things the WPF way for one reason or another. I agree we should question old ideas. However, we also don't want to restrict ourselves when perhaps it just wasn't understood why WPF needed to be the way it was.

[pr8x]

@robloo Wait so using Color? is an "ugly tear up" but adding some magic state to a general-purpose struct such as Color just to support a niche usage in ColorPicker is good? Ehhm, ok

Aligning is great, but adopting WPFs design mistakes should be avoided at all costs. Pretty sure that if you gave WPF folks a second chance for designing this Rect API they would decide otherwise. It seems like some legacy baggage they added at some point and never got rid of because "compatibility".

[YohDeadfall]

Over complication leads to various problems starting inability to limit input and ending by performance regression.

Let's take a look at Color. Any value of it in Avalonia is represented using ARGB, just a plain 32 bit unsigned integer. It fits well into registers and due to a lack of additional data can be used in vectorized computations. Add a new field to it and forget about single register storage, vectorization and other possible optimizations done by the just-in-time compiler.

From the consumer perspective, plain ARGB allows me to reuse that struct without inventing a wheel. Just a general purpose minimalistic which can be used to build another type by using Color as a field. If your API needs to distinguish between a null or some system-default value, add a custom enumeration type (in terms of F# or Rust, in C# it's not supported yet) using Color internally. That custom type can be an union inside for optimization purposes. In other words, that's up to the user how to build an application specific type.

The same applies to Rect.

While WPF was popular and made by Microsoft, it doesn't mean that everything is right here and anything around must be aligned with it. The same applies to UWP which has great ideas, but at the same time big issues too. That's why there are numerous frameworks and not a single silver-bullet.

[robloo]

You didn't address my example of double.NaN which is a good analogy. With your line of thinking double.IsZero is enough and double.NaN should never exist. Please consider this because when you understand why double.NaN exists and what it's used for its a good design.

The rest of the topic we are getting too theoretical and we are never going to align viewpoints I think so won't continue it further at such a high level.

The color example applies to Avalonia not to a separate app. This is a usability issue with the current ColorPicker control. I perhaps understand the tradeoffs in this example better than anyone.

[robloo]

Don't forget this is also an attempt to cleanup the current inconsistent API which uses Empty and Default with no thought for standardization or following WPF. If you are all OK with standardizing to Zero instead of Default/Empty I guess that solves the original problem well enough and I can accept the compromise with BoxShadow for now.

IsDefault, like double.NaN, does have clear added information and use cases though. So while it might not be added now I would still challenge you to think about the double case in my last comment. ColorPicker is yet an unsolved problem.

[pr8x]

NaN was introduced with IEEE 754 somewhere in the 80s. They needed some kind of a error channel for 0/0 etc and just used the exponent bits. C didn't have exceptions or any error handling at the time so it was good choice, but 40 years later? Not so much. God knows how many hard to track NaN bugs we could have avoided with a discrete error model for floating-point. So, no NaN is not a good example we should take inspiration from...

[robloo]

Lol, I think you will just say no to anything I say now.

Double.NaN is used extensively in Avalonia/WPF layout system. It basically means unconstrained height/width as im sure you know. This information is UNARGUABLY different than zero. While they could have used infinity instead, it's the same concept. More information is needed and zero doesnt work. You absolutely do not want to introduce nullability in the layout system.

[jp2masa]

I understand both points of view and honestly thought about this problem in the last few years, when it appears, and I don't have a strong opinion on the subject.

I don't know why "not a number" would ever mean unconstrained size. However, the performance difference appears to be very significant. So, this is basically a tradeoff between API and performance.

I have a proposal, but I imagine that the JIT and the corelib have lots of special cases for primitive types (float, double), which may make it less interesting (also, the effort to implement it):

For the example of NaN as unconstrained size, why not implement a custom struct around double? When it is a valid double, it has a value, but if not, it may encode other special values, which would be only unconstrained in this case. This case is so simple that it may even be something like:

public bool IsUnconstrained => double.IsNaN(_value);

public double Value => IsUnconstrained ? throw new InvalidOperationException() : _value;

This prevents incorrect handling of unconstrained sizes, while (hopefully) keeping performance.

For the example of Color, there could be a custom struct that encodes nullability as a special case, it would basically be a custom implementation of Color? as a struct, avoiding boxing and other problems.

This is the only way to keep good API and performance, but the tradeoff is now between API, performance, and maintainability.

[pr8x]

It's used because it's already there. When you have the luxury of designing a new API we should learn from past mistakes and do better. But in the end it's for the maintainers to decide this. I can only say our team (at work) agreed that the IsDefault change is misleading and should be reverted.

[robloo]

IsDefault change is misleading and should be reverted.

I wouldn't just revert it. The API consistency is still an issue. There were also a few minor fixes in that PR I believe.

Hopefully you consider IsZero acceptable to standardize with and we can close this discussion for now. It makes sense for numerical types even better than IsEmpty/IsDefault.

Keep in mind IsDefault already existed and will still exist in Avalonia for many other types, so saying you think it is misleading in the general case probably needs to be reconsidered. The API already had this as convention for the newer types -- especially text related.

[robloo]

@MikeCodesDotNet @maxkatz6 Can this be added to 11.0 milestone? It needs to be resolved one way or another before API release and it's on my todo list.

Either:

1. Revert API changes due to complaints and just leave it unorganized in this area and not matching WPF. Shouldn't revert the PR though as it had a few other fixes. As far as I can tell it's one or two strong complaints though, I don't know who is otherwise OK with it.
2. Add IsZero/Zero for numerical types and leave the rest alone. This still wouldn't fix the original inconsistent usage of IsEmpty and IsDefault though and we would now have Zero, Empty (for 1-2 types) and then Default.
3. Just use Zero and IsZero for everything that is checking for Zero properties (whether numerical type or not). That would replace Empty terminology. There will still be some other types using the Default terminology though related to font and some of the newer classes.

Of course I'll get to it myself but it will be a week or so and you'll have to accept the PR.

[maxkatz6]

Add IsZero/Zero for numerical types and leave the rest alone. This still wouldn't fix the original inconsistent usage of IsEmpty and IsDefault though and we would now have Zero, Empty (for 1-2 types) and then Default.

Point, Size, Rect - remove Empty/Default. In the future we can add Zero to support INumeric APIs from .NET 7. It's a good idea in general, but isn't useful before we implement these interfaces. Which won't happen in 11.0.

CornerRadius, Thickness - wpf doesn't have anything for these, I wouldn't add anything as well. Developers should use c# "default".

Color, Rect - remove Empty/Default as it's not consistent by behavior with WPF and Avalonia.

FontFamily - keep Default. Remove/make internal IsDefault (as suggested by @Gillibald).

Everything else (from original PR including boxshadow) - just use "default" C#, remove Empty/Default.

Discussed internally, and hopefully had an agreement.

[robloo]

@maxkatz6 Thanks for discussing it with the wider team.

For numeric types including Point, Rect, Size and Vector. Is my understanding correct:

1. Add IsZero property.
2. Do NOT add Zero field which will come with future INumeric APIs sometime in the future.
3. Remove/Obsolete Empty/IsEmpty/Default/IsDefault members. Default members that were added in last PR will be removed. Empty members that already existed in 10.x will be obsolete.

I think I understand the rest.

[maxkatz6]

1. No, how is IsZero better than comparing to default (or .Zero in the future)?
2. Yes.
3. We discussed that old members can be removed for 11.0 release as well.

[robloo]

No, how is IsZero better than comparing to default (or .Zero in the future)?

My confusion here is because of a few things, first, you quoted the part where I said to add IsZero to numerical types and leave the rest alone. That implied it is the direction you wanted to go. IsZero seemed to make sense for everyone using numerical types in the discussion above and is future proof as well.

Second, comparing to default (or the new IsDefault property) is ignoring the entire discussion above and the reason this issue (not the original one) was filed. Meaning is lost and code is, in some cases, harder to read than before. https://github.com/AvaloniaUI/Avalonia/issues/10264#issuecomment-1422326096. It is also harder to read when using default -- you have to be sure the type is clear or code is harder to read.

Finally, from a performance standpoint, I'm sure an instance doing integer comparison within itself for IsZero is better than any == default(T) check. Most numeric types are structs so we don't have to worry about allocation, but it still seems best to not create a new instance for this type of check. It's also possible for a type to implement IsZero better than the default comparison which will check using default comparer and all members.

[maxkatz6]

@robloo sorry for the confusion with the quote.

Finally, from a performance standpoint

From previous discussions, I still believe that there is no much meaning in the IsDefault or IsZero methods even from performance standpoint (at least for the generic structs).

Here is SharpLab which runs on .net 7 in release mode in this example: https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEBDAzgWwB8ABAJgEYBYAKGIAYACY8gOgBEBLbAcwDsJcGDmFwsAwhAAmMAIK9sAGwCeuDrgDcNGsQDMTUgzE0A3jQbmme5kgbAIEBQwAqMQWxgAzbAFcFGABQAlGYWptQWEUwA7AwAkrjuXr4Y5P7SSX6BmuEWAL4h5rpM5DZ2Ds6uGAByELyJPn5BBQxhkRbEMfH1yam8MADu/uRoDOSBWc35OYVWJbb2jl2eDSn+AAoQHLwYDAAOwdMtzREdewwAvOcM6SvZEVMns6ULcQnLyaTrm9t7BxGtbWiexYSwyGDueRoU20elgkCgkgYgig3jAOw2WwCmIYAA0RtiAJoHAEzeblUErC4APlxFyujAAZAyGAS6Qw6NlckA==

You can see that whole process of creation an empty struct (from "default" keyword) and Equals methods execution is eliminated from the code and inlined to the minimal construction. There is still a difference in the code gen, as "p == default" generates couple of unnecessary moves, and instead of "cmp" it uses "test" operation.

Note, I used "record" only so I don't need to write equals and operator methods. Obviously, by default simple "struct" comparison would be very slow because how .NET runtime compares structures without overriden Equals method (badly). That's why in

Also interesting part, "p == default" code is simple enough to inline comparison result into a constant, if "p" is known compile time (see code gen for TestDefault and TestNonDefault methods).

Now, if we change target to .NET Framework, it will perform way worse for sure. But I don't think we care that much about .NET Framework. As it's pretty much well known fact that .NET Core is faster.

Upd: for clearness, copy pasted Point code from our repository. It looks like there is zero different between "== default" and ".IsDefault" now.

[maxkatz6]

It's also possible for a type to implement IsZero better than the default comparison which will check using default comparer and all members.

This is correct. But it's not the case for most of these types. BoxShadow maybe, but it's better to just remove "default/empty/zero" from there at all.

Meaning is lost and code is, in some cases, harder to read than before

Yes, and that's why removing these (and not just obsoleting) members would help to uniform code with the ecosystem and reduce confusion, when there is only one "default" way to do comparison.

[robloo]

@maxkatz6 Yes, I recall our older discussions. At this point I'll just say it makes me uneasy relying on compiler optimizations like this. However, I've been stuck on old .NET versions so long I don't have a good feel for this area. I was also talking more generally as this API could apply to classes as well as structs but all cases right now are just structs (aside from the font-related areas). If the compiler will optimize as you described I have no concerns.

Yes, and that's why removing these (and not just obsoleting) members would help to uniform code with the ecosystem and reduce confusion, when there is only one "default" way to do comparison.

As you said, removing all inconstant members and relying on the language default keyword will allow us to standardize now while giving us freedom to improve in the future if needed (around Zero terminology). Until then we do have consistency. Keep in mind there are two areas this can open again in the future:

1. For classes (any reference types)
2. For numerical types implementing INumeric... Vector really should have this at a minimum.

The readability issues as pointed in this issue itself will have to be dealt with outside of my hands. I believe you have a direct dialog through separate channels as necessary.

Note, I used "record" only so I don't need to write equals and operator methods. Obviously, by default simple "struct" comparison would be very slow because how .NET runtime compares structures without overridden Equals method (badly).

Just to be sure, I checked with a regular (non-record) struct and implemented all the equality operators and methods. It compiles the same as the record so there is no concerns. I think all Avalonia types in question already do this and if not I will add it.

Code to test a regular struct.

## Paste in SharpLab ```csharp using System; using System.Diagnostics.CodeAnalysis; public class C { public static bool TestDefault() { return IsDefault1(default); } public static bool TestNonDefault() { return IsDefault1(new(1, 1)); } public static bool IsDefault1(Point p) { return p == default; } public static bool IsDefault2(Point p) { return p.IsDefault; } public static bool IsDefault11(Vector vector) { return vector == default; } public static bool IsDefault12(Vector vector) { return vector.IsZero; } } public record struct Point(int X, int Y) { public bool IsDefault => X == 0 && Y == 0; } public struct Vector : IEquatable { public int X { get; } public int Y { get; } public bool IsZero => X == 0 && Y == 0; public bool Equals(Vector other) { return X == other.X && Y == other.Y; } public override bool Equals(object? obj) { if (obj is Vector passedObj) { return Equals(passedObj); } else { return false; } } public static bool operator ==(Vector left, Vector right) { return left.Equals(right); } public static bool operator !=(Vector left, Vector right) { return !(left == right); } } ```

[robloo]

I'm going to open up a PR with these changes by the end of the day. Hopefully it can slip into the next preview if that is coming soon.

[robloo]

Important

Note for everyone. It makes sense going forward to be explicit in code when using a Rect struct on what the comparison is actually doing. Writing aRect == default or even the older aRect.IsEmpty largely ambiguous as partially discussed before. However, there is a more important consideration: In most code you want to know if the height/width ONLY are zero (ignoring X/Y position). This is especially true in rendering paths.

The older IsEmpty and IsDefault implementations only checked that height/width were zero. So they were "correct". However, code is never clear about this and going forward doing only an == default check is technically a breaking change (and maybe a bug) as X/Y position would also be checked which may be unwanted.

Bottom line: Be clear and always do a check for Width/Height == 0 when appropriate. Code is then more readable and won't need to change in the future. It also no longer has any ambiguity relying on hidden implementations.

---

This should be added to the 11.0 breaking changes summary later.

[kekekeks]

Note: WPF's Rect.Empty does not match default and has some special handling. I'm not sure if we want to switch to that now, but it's something we need to consider too.

[robloo]

@kekekeks Yes, that was discussed a but above. https://github.com/AvaloniaUI/Avalonia/issues/10264#issuecomment-1422883614. There was push back about keeping the struct simple. My retort was https://github.com/AvaloniaUI/Avalonia/issues/10264#issuecomment-1450396315.

Code now is explicitly checking height and width for Rect so even if we decide to match Rect in the future with WPF code shouldn't need to change. It's a little future proofed.