Pos/Dim Thoughts and Exploration for V2 (and/or beyond)

[dodexahedron]

What follows is all related to Pos and Dim, but are more like ideas and discussion prompts. None of these things are bugs, but rather are design details that I think are worth considering. Some are based on C# design guidelines, and some are more opinionated, based on my experience with TG and TGD from primarily the consumer side, but also the maintainer side.

Soooo let's begin with a whopper!

• All of the Pos and Dim types really are value types.
  • Except for the inheritance of a couple of members and implicit casting from more derived types to the base Pos type, why are these reference types at all?
    • The inheritance can be solved with interfaces.
    • Equality operators are not implicit on non-record structs, so the accidental inconsistency goes away right away.
    • A readonly record struct would be immutable and have implicit value equality of all members auto-generated by the compiler.
    • I know a concern was voiced before about reflection at one point as a reason to avoid making anything any kind of struct, and I didn't say anything then because I wanted to check myself first, but what is the actual concern there?
      • In dotnet land, structs are still just another kind of Type and can be reflected on like anything else. Properties, fields, methods, attributes - you name it - it's all there, just as with a reference type.
      • You just need to be aware that you are dealing with a value type, with everything that means[^2], since the compiler isn't doing it for you to the degree it does when using first-class non-reflected value types.
      • All you have to do to be sure you don't lose implicitly-made copies of mutated immutable types, during reflection, is just box the struct before accessing it, either in an interface or even just with plain ol' object. The reflection methods are all going to return object anyway, so this isn't even a hit to performance. Boxing it yourself makes the compiler grab it and deal with it like you're used to dealing with any other boxed value type. Ideally, if there's a public interface defined that the struct implements, you can even just cast it to that once you have the struct, and just interact with it strongly-typed from there (just as you would with a reference type).
    • With record structs[^1], you are able to use the with operator for non-destructive mutation, when needed. That syntax is hella nice and is nearly identical to regular object initializers. The difference is the compiler generates the copy constructor for you (by-value copy constructor), and the with statement lets you change as many or as few properties as you like, including just an empty block (myRecordStructInstance with {}) which is just a straight-up copy.
      • Some people call the copy on these a "shallow" copy and the equality on these "shallow" equality.
        • It's not.
        • The auto-generated equality on a record struct is the most literal interpretation of by-value equality there is, in the context of c#. Value type members are compared by value, without reflection, and reference type members are treated as if the reference itself is the value (not the object it points to), so they have the "value" of their references compared (which is to say that's reference equality, which is what should be expected for a reference type member of a value type).
        • The copy works similarly to the equality, but for copying instead of comparison.

A structural thing, but from a more pragmatic view of current implementation, where benefits can be realized with surprisingly little actual work:  

• I really can't find a reason for the various Pos and Dim types to be both nested classes and descendants (ie inheritors) of their parent types, as they currently exist.
  • Nested classes of Pos and Dim need no direct instance-level access to any members of their parent types.
  • The nested classes inherit from their parent types anyway, so any access they need can be handled via inheritance.
  • There's a small amount of grey area but, for the most part, everything about those being nested types goes against the C# and dotnet design guidelines (nested types aren't CLS-compliant, for one)
    • It's also, as currently implemented, completely redundant with inheritance, but brings along baggage that solely using inheritance doesn't have.
  • Just as proof of concept, using the current v2_develop branch as base, I promoted all of the nested types of Dim and Pos to internal classes, just to see how badly that would break everything.
    • ReSharper took care of removing the class qualifiers, which were the only thing different between them being nested and not.
    • All tests continued to pass
    • Took my 4 year old laptop about a minute and a half to scour the entire solution for references and fix them all up automatically.
    • All I had to do manually was add some usings, which I did via global usings in the project files, and that's only because I also moved them to their own namespace as a further attempt to confuse and break things.
      • The attempt to confuse and break things failed - the project, samples, and tests all continued to build and pass.
    • This was an independent PoC from the value type proposal. They're still classes in this modified code, but Pos and Dim are abstract and their previously-nested classes are all now peers, just inheriting from the abstract bases.
    • The generated assembly in debug mode was about 8kB smaller than the original, with just that set of changes being made. It does touch 150 files, though (most are just removing now-invalid using statements).

And, as an even less impactful change/thought (which I've also tried out):

• Pos and Dim themselves, in their current state (ignoring the value vs reference topic and everything else above), really are effectively abstract classes that just don't have abstract keywords on them.
  • Users can't instantiate them directly, in a meaningful way. The default constructor for Pos has no parameters and can only yield a basic Pos object, which is pretty useless on its own, not having any local concept of a value by itself, and if instantiated directly via new Pos() can't be cast to the other types, because it's the base and covariance doesn't work that way without generics, which aren't relevant here.
  • Just how sure am I that Dim and Pos can be abstract? Very! Simply adding abstract to the Pos class required no other changes to the code to build and pass all tests (aside from the 2 that just call the constructor and nothing else).
  • Even making Anchor be abstract instead of just virtual, to force descendants to implement it, caused no issues, and proved to me that Pos.Anchor always yielding zero isn't actually a problem because it never happens and can't ever happen anyway. Plus, it's internal, so users can't call it without reflecting anyway, so it's a prime candidate for refactor.
  • Those two classes are so abstract they almost should just be interfaces. I could make an argument both ways for that though, and I usually do both in projects, because abstract bases and interfaces each have value/use cases, and the effort of making an interface in addition to an abstract base class is basically zero (literally just a few mouse clicks, using ReSharper). Makes mocking for testing easier, too.
  • If their nested types were to be turned into value types, of course the abstract base class option goes away, since a struct can't inherit from a class and vice versa, but interfaces are still there and can be used instead.

And then going beyond TG itself, into tools that consume it and generate code using it, such as TGD:

• Have we ever considered not even having different types for those, and just doing something more similar to how WPF, WinForms, or even HTML/CSS handle it?
  • Just using WPF as an example, size and position are basically a conditional conglomeration of the following, within a given non-flow container:
    • Explicit width and height, as highest priority for size. If not specified or only partially specified, missing components of it are calculated based on everything else.
    • Alignment enums (which are essentially analogous to a combination of what XCenter, XAnchor, and other similar types in TG do)
    • Absent those, or in conjunction, as appropriate:
      • Margins
      • Padding
      • Borders
      • A concept of Z-ordering
      • And that's about it, for the most part. There are other more esoteric things, but Margin and alignment cover most cases, and Width and Height on top of those must get you to like 99+% of use cases. And then when we consider this is a TUI library, with inherently simpler layouts, more than that probably isn't necessary.
    • I'd likely limit support to explicit sizing
  • I've had ideas, for some time now, about adapting a very stripped down version of XAML as a markup language that can be translated to Terminal.Gui, largely based on some concepts in TerminalGuiDesigner, but complexities such as the current Pos/Dim implementation[^3] make that a daunting task. [^1]: Or regular structs or record classes, but I'm focusing on just one thing here or this will get even bigger than it already is. [^2]: Such as potential immutability and the consequences of that like copy-on-write (I say "potential" because you can make mutable structs, if you like to be crazy. Also, record structs are mutable by default for some reason I can't fathom, unless you declare them as readonly record struct) [^3]: Plus details such as type and member visibility, which would either need broader exposure or (less ideally) require proxy/wrapper types to fake everything via reflection and other runtime trickery against Terminal.Gui under the hood or even not using Terminal.Gui at all until the actual creation of syntax trees and such for C# code generation. Obviously I'd much rather duplicate as little effort as possible, especially since this project represents years of several people's fantastic hard work.

[tig]

I'm not smart enough yet to really grok all of this.

I'm also not confident in our Pos/Dim/SetRelativeLayout/LayoutSubviews unit test coverage.

Therefore, my vote is "Go for it! But before you do, we must significantly up our unit test game in this area."

Also, wait until I get Dim.Auto done so I'm not working on a moving target. https://github.com/gui-cs/Terminal.Gui/pull/3062

In my work on Dim.Auto so far I have found several places where I don't think anyone understood the codebase. I found several cases that were clearly broken, but nobody noticed (e.g. Pos.Center() + Pos.Absolute())). There's another dragon lurking that I have a bad feeling about having to do with Dim.Fill and Pos.Center...

[dodexahedron]

I'd be happy to work on some TDD for those types, once I'm finished with the work I'm doing with TerminalGuiDesigner's test project.

They're pretty critical core classes, so I wholeheartedly agree they should be tested excessively. :)

I imagine the work on the TerminalGuiDesigner test project will probably take me another week or two, if I'm able to keep up my current pace.

[tig]

See

• #3410

It makes progress on cleaning up these classes.

[tig]

Now that #3480 is merged and #3415 is almost ready, it would be awesome if this Issue were updated with a clear list of the remaining issues you have related to all this.

[dodexahedron]

Yeah I actually think this one is basically obsolete, as some things have changed over time.

The fundamental thoughts are still more or less the same, but this was originally intended to be more of a discussion and brainstorming prompt than anything.

For actual work, smaller bite-sized pieces like the interface issue draft I started are more my intent for this. Although that one isn't exactly tiny either. But it's still not really terribly complex - especially if I whiteboard it out to better visualize stuff.

I may actually shift to that stuff for tomorrow, since EV validation for the new org code signing cert is going to take a bit of time and then the physical usb token/essentially smart card (FIPS 140-3...woooo... 🙄) is sent by FedEx ground from Ontario to Arizona once that's all done. So I won't be publishing refactored NuGet packages for a few more days, still, at least, while I'm waiting on that.