Roslyn: Concerns about the Deconstruct method

[mharthoorn]

I feel that the current feature planned in C#7 of deconstructing a class into a tuple by using a Deconstruction method with _out_ parameters has some serious limitations.

1 - To be able to serve all use cases, you may need to create multiple Deconstruct overloads, some of which might never be used.

2 - The Deconstruct functionality cannot be used in expressions, (fluent) method chaining or parameters.
It basically doesn't interact well with any other aspect of the language. Consider a Point with a Deconstruct(out int x, out int y) method,

        public void Negate((int x, int y) coord)
        {
            ...
        }
        ...
        Negate(new Point(1, 2)); // cannot convert point to (int x, int y)

3 - Classes that have multiple fields or properties of the same type have a high risk of overlapping Deconstruct overloads.

    public class Rectangle 
    { 
        public int X; 
        public int Y; 
        public int Width; 
        public int Height;

        public void Deconstruct(out int x, out int y) { x = X; y = Y; }

        public void Deconstruct(out int width, out int height)
        { width = Width; height = Height; } 
        // error: already defined with same parameter types
    }

[stepanbenes]

Do you also propose some alternative approach?

[mharthoorn]

@stepanbenes, since tuples by their very definition have contextual value only, i believe an ad hoc deconstruction makes more sense. Basically any syntax that
(a) can make a deconstruction part of an expression and
(b) let the consumer of the deconstruction determine the order and selection of fields/properties

I did propose a dot syntax for inlineable deconstruction a while a go: #13385. But I can imagine there are more traditional notations possible.

[DavidArno]

"Deconstruction is positional". This is a common mantra that one reads around here. This led me to a completely false conclusion that, for the following code example:

public class Rectangle
{
    public int X;
    public int Y;
    public int Width;
    public int Height;

    public void Deconstruct(out int x, out int y, out int width, out int height)
    {
        x = X;
        y = Y;
        width = Width;
        height = Height;
    }
}

, I could then do the following:

var rectangle = new Rectangle();
var (x,  y) = rectangle;

It's positional, so I'm saying I want the first two items from a deconstruction of Rectangle. But that gives a compiler error that I'm missing width and height.

As @mharthoorn points out, if I want just X and Y, I need to create a two parameter Deconstruct method. What then if I want Width and Height? On top of that, deconstruction is syntactic saccharin, in that it looks sweet but the the need to explicitly create Deconstruct methods, along with the lack of discoverability and the use of out parameters, leaves a bitter aftertaste.

I may well eat my words in future, but at the moment the deconstruction feature looks like a non-starter to me that I'd actively avoid using. I'm more likely create a Rectangle type like the following:

public class Rectangle
{
    public int X;
    public int Y;
    public int Width;
    public int Height;

    public (int x, int y) Position => (X, Y);
    public (int width, int height) Location => (Width, Height);
    public (int x, int y, int width, int height) AsTuple() => (X, Y, Width, Height);
}

In less code, and without having to use out parameters, I now have a set of fully discoverable properties/methods that are making clear what decomposition they offer. All without invoking some "magic compiler hack" that Deconstruct has to use.

The above, coupled with @mharthoorn's excellent dot notation idea for property patterns:

var (x, width) = rectangle.(X, Width);

offers all I think I'd need for creating tuples from other types.

[HaloFour]

Deconstruction is explicitly intended to be the inverse of construction, which is positional and overloadable based on the signature of the parameters. I do believe the goal is to also make them conditional so that they can participate in pattern matching by making the return parameter bool.

Property patterns will cover nominal deconstruction, with the following syntax assuming that let is implemented as it was proposed:

let { X is var x, Width is var width } = rectangle;

[MgSam]

I have to agree with @DavidArno's sentiment. The Deconstruct method just isn't worth the cost/benefit. I can't see myself ever writing (or even using a tool to generate) this method. Maybe its fun from a language design perspective, but from a usage perspective its just a lot of heavyweight language baggage with not much benefit.

[alrz]

@MgSam This is the internal mechanism used by records to provide positional deconstruction. I think there is no reason to not expose it to the developer, i.e. one can use an extension method to implement positional deconstruction for external types.

@mharthoorn You can use wildcards for ignoring variables or just use property patterns.

[MgSam]

@alrz Except- records don't exist yet. So should we be adding this piece to the language now when there's no guarantee records will ever be in the language? It's well and good that it's "planned" for C#7+ but we all know how that goes... Priorities change, the language team moves onto something else.

In my view, it's unlikely records will ever happen. The team is spending a huge amount of effort adding tuples for C# 7, and they handle most of the use cases of records.

[DavidArno]

@HaloFour

Deconstruction is explicitly intended to be the inverse of construction, which is positional and overloadable based on the signature of the parameters.

Without records, this is a meaningless statement. Those "parameters" are specific to records. For other types, they simply become an obfuscated method for creating arbitrary tuples from arbitrary aspects of the type, with no correlation with the constructor required. And records aren't planned for C# 7, but "magic" Deconstruct support is, which begs the question "why?" The language would be more consistent if Deconstruct were held back until records are introduced.

I do believe the goal is to also make them conditional so that they can participate in pattern matching by making the return parameter bool.

And since real pattern matching has been dropped for C# 7, it makes no sense to half-implement Deconstruct with just void support.

[HaloFour]

@DavidArno

I would completely agree that having the tuple features before records and pattern matching seems like putting the cart before the horse. We've seen the same argument regarding wildcard syntax in tuple deconstruction.

I am optimistic that the bits of implementation we are seeing now are there specifically to lay the groundwork towards the larger set of features. If they don't then I would agree that the features as currently designed aren't exactly optimal for the specific purpose of working with tuple-likes. My hope is that with the major language features in place that it's a "minor" change to extend "type-switch" to support conditional recursive pattern matching, possibly even something that could be shipped in the 7.x timeframe rather than waiting until 2018ish.

[DavidArno]

@HaloFour, I guess we will just have to wait and see what happens as the language team are unlikely to commit to anything in advance.

[HaloFour]

@DavidArno Agreed. If it would put minds at ease and allow the design team some more flexibility with pattern matching I would not be opposed to the team deciding to defer the ability to deconstruct arbitrary types via this Deconstruct method. It would really suck for the team to realize after the fact that they missed something in the design but are now stuck with it.

[jcouv]

@mharthoorn I will go ahead and close the issue, as C# 7.0 has shipped. Feel free to re-open if needed. Thanks