Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support delete member in C# dynamic object #16321

Closed
Thaina opened this issue Jan 7, 2017 · 50 comments
Closed

Support delete member in C# dynamic object #16321

Thaina opened this issue Jan 7, 2017 · 50 comments

Comments

@Thaina
Copy link

Thaina commented Jan 7, 2017

Since DynamicObject support TryDeleteMember and dynamic object mostly used to bind with json and other remote system which sometimes treat null and not exist differently

I think we should support delete syntax on dynamic object

maybe just use ~

void Main()
{
    dynamic obj = LoadJson("config.json");
    ~obj.MustDeleteData;
    obj.NullableData = null;
    SaveJson("config.json",obj);
}
@HaloFour
Copy link

HaloFour commented Jan 7, 2017

~ is already an operator in C#.

@Thaina
Copy link
Author

Thaina commented Jan 7, 2017

Well thank you. I forgot

Then how about

obj.MustDeleteData~;

@HaloFour
Copy link

HaloFour commented Jan 7, 2017

Honestly if a language feature were to be added specifically to permit this behavior I'd prefer that delete be made a contextual keyword so that C# could follow the EcmaScript syntax:

void Main()
{
    dynamic obj = LoadJson("config.json");
    delete obj.MustDeleteData;
    obj.NullableData = null;
    SaveJson("config.json",obj);
}

At the same time I'm not aware of a direct way in C# to set a property to undefined, which is different than deleting a property:

void Main()
{
    dynamic obj = LoadJson("config.json");
    delete obj.MustDeleteData;
    delete obj["MustAlsoDeleteData"];
    obj.NullableData = null;
    obj.UndefinedData = undefined;
    SaveJson("config.json",obj);
}

Although perhaps both could be accomplished through static members exposed on some helper class which would negotiate the dynamic object and manually manipulate it. I'm not sure that would be possible with the delete keyword as described above since the property would be resolved and passed to the method, but perhaps the method could accept the key name:

void Main()
{
    dynamic obj = LoadJson("config.json");
    DynamicHelper.delete(obj, "MustDeleteData");
    obj.NullableData = null;
    obj.UndefinedData = DynamicHelper.undefined;
    SaveJson("config.json",obj);
}

With using static you could reduce that to just delete(obj, "MustDeleteData"); and obj.UndefinedData = undefined; respectively.

Note that I'm aware that ExpandoObject does allow for deleting properties through casting to an IDictionary<string, object> and calling the Remove but that behavior is specific to the implementation of that dynamic type.

@Thaina
Copy link
Author

Thaina commented Jan 7, 2017

I think = undefined and = delete is not possible because there would someone already uses it as a variable name

So I just think ~ is more safe to use. It like a shorthand to your = Dynamic.undefined

@HaloFour
Copy link

HaloFour commented Jan 7, 2017

Which is why I said they could be contextual keywords, as in the kind that aren't keywords if a variable or type of that name is in scope.

~ doesn't fit in the syntax, not in C# nor in EcmaScript. Also, setting a property to undefined and deleting a property don't do the same thing in EcmaScript.

@iam3yal
Copy link

iam3yal commented Jan 7, 2017

I'm baffled why would you use dynamic for a structured data? why does LoadJson return a dynamic object?

@jnm2
Copy link
Contributor

jnm2 commented Jan 7, 2017

I'm baffled why would you use dynamic for a structured data

How structured is it if he's deleting a property? 😆

@iam3yal
Copy link

iam3yal commented Jan 7, 2017

@jnm2 The question should be why it isn't structured? if something is removed from it I'd argue there's some garbage that shouldn't be there at all.

p.s. The config.json file is probably structured and you can create a structure that represents it.

@jnm2
Copy link
Contributor

jnm2 commented Jan 7, 2017

@eyalsk Suppose the structure by design isn't known at compile time?

@iam3yal
Copy link

iam3yal commented Jan 7, 2017

@jnm2 Here are few examples/ideas:

public class Config
{
	public string Property1 { get; set; }

	public string Property2 { get; set; }

	public dynamic Bag { get; } = new ExpandoObject(); // Used for some trash data
}

Then when you serialize it you can ignore the Bag property.

Alternatively you can create a wrapper around the dynamic object.

public class DynamicWrapper
{
	public DynamicWrapper(dynamic data)
	{
		DynamicObject = data;
	}

	public dynamic DynamicObject { get; }
	
	public void Delete(string key)
	{
		(DynamicObject as IDictionary<string, object>).Remove(key);
	}
}

public DynamicWrapper LoadJson(string json)
{
	dynamic config = new ExpandoObject();
	
	config.property1 = "prop1";
	
	return new DynamicWrapper(config);
}

Usage:

DynamicWrapper x = LoadJson("config.json");

x.DynamicObject.Property1 = "whatever";

Console.Write(x.DynamicObject.Property1);

x.Delete(nameof(x.DynamicObject.Property1));

Console.Write(x.DynamicObject.Property1); // 'System.Dynamic.ExpandoObject' does not contain a definition for 'Property1'

@HaloFour
Copy link

HaloFour commented Jan 7, 2017

@eyalsk

As I mentioned above, the ability to cast dynamic to IDictionary<string, object> is a specific implementation feature of ExpandoObject. You can't treat any arbitrary dynamic instance that way. If LoadJson is backed by some JavaScript engine via DLR it's likely that approach will not work.

@iam3yal
Copy link

iam3yal commented Jan 7, 2017

@jnm2

Suppose the structure by design isn't known at compile time?

Are you speaking in general or in this specific scenario? because here he/she loads something that seems to be well known at compile-time.

@HaloFour You're right it's implementation dependent but then you can still have the same wrapper and then instead of calling Remove on the ExpandoObject make a P/Invoke call that goes to this engine and removes it from the object.

Wouldn't a contextual keyword that deletes the property would have to be implementation dependent in the same way? I mean it will have to go to this JavaScript engine and call delete to really remove it from the object, isn't?

@HaloFour
Copy link

HaloFour commented Jan 7, 2017

@eyalsk

Wouldn't a contextual keyword that deletes the property would have to be implementation dependent in the same way? I mean it will have to go to this JavaScript engine and call delete to really remove it from the object, isn't?

No, the DLR API has support for deleting members. C# just lacks a mechanism to expose using it.

@iam3yal
Copy link

iam3yal commented Jan 7, 2017

@HaloFour Okay, I didn't know that, now I do. 😆

@jnm2
Copy link
Contributor

jnm2 commented Jan 7, 2017

From my perspective, a delete language concept seems too fundamental to implement just for the sake of dynamic. If it was me, I'd use LINQ to JSON over dynamic any day. But if it made sense in contexts besides dynamic...

@iam3yal
Copy link

iam3yal commented Jan 7, 2017

BTW, didn't realize the OP speaks about API that available to DynamicObject even though it's clearly written, I just looked at the API and read the post again and yeah... my bad.

@Thaina
Copy link
Author

Thaina commented Jan 9, 2017

@jnm2 I think opposite. dynamic is really a massive keyword. It cover every object in DLR. It's like object class that represent everything in C#

So a keyword for the sake of dynamic is more important than LINQ

But I agree that if there are delete keyword I would like it to work with other things too

@Thaina
Copy link
Author

Thaina commented Jan 9, 2017

If we could use contextual keyword there I think reuse remove keyword would be better

void Main()
{
    dynamic obj = LoadJson("config.json");
    remove obj.MustDeleteData;
    obj.NullableData = null;
    SaveJson("config.json",obj);
}

Also there are one thing I would like to, that delete or remove keyword should return object that was being deleted, like Stack.Pop()

void Main()
{
    dynamic obj = new DynamicObject();
    obj.Number = 1;
    int i = remove obj.Number; // i became 1 and Number was deleted from obj
}

If it possible I would like to have this behaviour on List and Dictionary

void Main()
{
    Dictionary<string,int> dict = new Dictionary<string,int>();
    dict["Number"] = 1;
    int i = remove dict["Number"]; // i became 1 and Number was deleted from dict
}

@iam3yal
Copy link

iam3yal commented Jan 9, 2017

@Thaina

So a keyword for the sake of dynamic is more important than LINQ

Probably to you. :)

I'm not in favour of adding anything to dynamic because I don't use it as much, the only place I used it is in ASP.NET MVC 3 or 4 can't remember, I really prefer features added to LINQ any day...

But I agree that if there are delete keyword I would like it to work with other things too

It might make sense for dynamic but I don't think it would make sense to introduce this for other collections.. we already have APIs for it so what's the value/benefit here?

@Thaina
Copy link
Author

Thaina commented Jan 9, 2017

@eyalsk We don't really need linq keyword because we could access linq in extension method style. Even we add more feature to linq it more likely is extension method anyway

But dynamic is not. dynamic cannot assume any member to work and be the same API. So operator to work with it is really needed

don't think it would make sense to introduce this for other collections.. we already have APIs for it so what's the value/benefit here?

I think it like we have linq keyword. collection.Select(() => x.Thing) could be written as select x.Thing from Collection

So it the same for collection.Remove("X") should be able to write as remove collection["X"]

But yeah this is so very minor

@iam3yal
Copy link

iam3yal commented Jan 9, 2017

@Thaina

We don't really need linq keyword because we could access linq in extension method style. Even we add more feature to linq it more likely is extension method anyway

You misunderstood me, I didn't say I want them to introduce the linq keyword into the language, I said that given a decision between these two features I'd prefer them to add more features to LINQ than adding this feature.

LINQ isn't just a bunch of extension methods, there are types related to it, expressions, language integration, etc...

But dynamic is not. dynamic cannot assume any member to work and be the same API. So operator to work with it is really needed

Maybe it's really needed, let's assume it is, the question is how many gonna need it?

I think it like we have linq keyword. collection.Select(() => x.Thing) could be written as select x.Thing from Collection

Don't you mean from x in collection select x.Thing?

@Thaina
Copy link
Author

Thaina commented Jan 9, 2017

@eyalsk Almost all about linq is extension method on IEnumerable<> and lambda. Type relate to it is mostly just tuple if we have tuple in those day

And linq expressions from where select etc is really not needed

And what is language integration you mean? Linq is extension method so it is the same class citizen as all other in C#

how many gonna need it?

Almost all dynamic object used for interop with dynamic language is needed. Well I think in present day we do so many workaround like JObject but actually it should be replaced with dynamic and treated like in js

Don't you mean from x in collection select x.Thing?

Right

@Thaina
Copy link
Author

Thaina commented Jan 9, 2017

@eyalsk My point is, any linq feature mostly could be added in dotnet/corefx And don't need to add anything in dotnet/roslyn, unless you need to add keyword, which I don't think it need anyway

Or are there any feature you really want on linq that cannot extend by extension method?

But to expose delete on dynamic is another story. There are no clean way to do beside changing compiler

@iam3yal
Copy link

iam3yal commented Jan 9, 2017

@Thaina

And linq expressions from where select etc is really not needed

Just because you think it's not needed doesn't mean it doesn't exist as part of the language, you can check the spec it's mentioned under Query Expressions.

Almost all dynamic object used for interop with dynamic language is needed. Well I think in present day we do so many workaround like JObject but actually it should be replaced with dynamic and treated like in js

Why? can't you just do this:

dynamic person = new JObject();
person.Name = "John";
Console.WriteLine(person.Name);

@iam3yal
Copy link

iam3yal commented Jan 9, 2017

@Thaina

Or are there any feature you really want on linq that cannot extend by extension method?

I don't really need anything atm but there are many issues about people asking more features added to LINQ:

#15619
#13919
#8221
#6877
#3571
#3486
#100

These are just some features but there are many more from syntactic sugars, adding existing language features that currently don't play well with LINQ to performance so it's not just about adding functionality.

@jveselka
Copy link

jveselka commented Jan 9, 2017

I don't think keyword is necessary. Why not just update BCL to have something like this?

public static class DynamicHelper
{
  public static void RemoveMember(object dynamicObject, string memberName);
  public static bool TryRemoveMember(object dynamicObject, string memberName, out object removedMember);
}

Its by far more simple solution to implement (if I understood correctly, it can be implemented directly in CLR without changes to any spec).
We can have multiple overloads without arguing if it should or should not return removed member, throw or do not throw when the member does not exist etc.

@Thaina
Copy link
Author

Thaina commented Jan 10, 2017

The code example you write is

dynamic person = new JObject();
person.Name = "John";
Console.WriteLine(person.Name);

Yes, this is what is should be, but then how can you delete Name after that?
Currently now it must

dynamic person = new JObject();
person.Name = "John";
Console.WriteLine(person.Name);
((JObject)person).Remove("Name");

That's why I want to propose that it should be just

dynamic person = new JObject();
person.Name = "John";
Console.WriteLine(person.Name);
remove person.Name;

To make dynamic flow can complete by itself. That's what I mean

@Thaina
Copy link
Author

Thaina commented Jan 10, 2017

I don't really need anything atm but there are many issues about people asking more features added to LINQ

Almost all that was needed to extend corefx. Some is just waiting C#7 for return ref or tuple. Nothing directly relate to linq anymore except 2 request for adding more keyword just for linq which I think is bad idea

@Thaina
Copy link
Author

Thaina commented Jan 10, 2017

@zippec That's not a clean way to do. What you said is like, class is syntactic sugar, we don't need class syntax at all. We could workaround by making all function has classname_ in front of it

Currently now we can workaround with anything, that's true, and it's not a blocking issue
But proposal is what we made for make things better in the near future

@iam3yal
Copy link

iam3yal commented Jan 10, 2017

@Thaina If anything the keyword should probably be dynamic_remove or something and not as generic as remove.

Almost all that was needed to extend corefx. Some is just waiting C#7 for return ref or tuple. Nothing directly relate to linq anymore except 2 request for adding more keyword just for linq which I think is bad idea

So adding keywords to a common feature such as LINQ is in your opinion bad but adding a keyword to an uncommon feature is better?

@jnm2
Copy link
Contributor

jnm2 commented Jan 10, 2017

I think that a generic delete concept would be nice to explore for the sake of dynamic (though I'd never use it) as well as other parts of the language. Maybe it could be an operator that dictionaries could use. Maybe we could undefine locals or have deterministic freeing. Maybe it could be used for unsafe operations.
The C# team wanted to explore adding a move keyword. This doesn't feel that different.

@iam3yal
Copy link

iam3yal commented Jan 10, 2017

@jnm2 In C++ the delete keyword is used in more than one context but the thing is I'm not sure whether the use-case is convincing enough to introduce this keyword into the language.

How many people use delete obj.key in JavaScript? how many people use delete obj in C++? not many in fact in both of these languages it's often a bad practice for different reasons but still.

Maybe it could be an operator that dictionaries could use.

Maybe but there's some debatable questions to ask:

  1. I assume keys will have to be known at compile-time but what if the key is not a valid identifier?

  2. Would it support user-defined types? like a custom dictionary?

  3. Would it be a syntactic sugar to Remove<T>(T key) or only to Remove(string key)?

Maybe we could undefine locals or have deterministic freeing

Check this #161.

What do you mean by deterministic freeing? do you mean something like delete SomeObject would call Dispose given that it's implemented? but then how is it different than the using statement?

The C# team wanted to explore adding a move keyword. This doesn't feel that different.

Well, the use-case is very different because it actually related to #161 as noted there so there is a compelling reason to add it whereas here it seems reasonable but again how many people are going to use it?

@jnm2
Copy link
Contributor

jnm2 commented Jan 10, 2017

do you mean something like delete SomeObject would call Dispose given that it's implemented? but then how is it different than the using statement?

More like calling the finalizer, but to answer all your questions: I'm not making a proposal, I'm trying to get people's imaginations going and see if there is anything that delete would be cool for.

@iam3yal
Copy link

iam3yal commented Jan 10, 2017

@jnm2 Yeah I figured, I just thought to bring some more points to the discussion when considering the merits of this feature/keyword. :)

@Thaina
Copy link
Author

Thaina commented Jan 11, 2017

I was thinking about using it with any collections. And idea that was popping up is, we should introduce remove keyword to indexer. And make interface to be implemented explicitly on current collection we have

public interface IHasIndexer<K,V> : ICollection<T>
{
    this[K key] { get; set; remove; }
}

System.Collection.Generic.List<T> : .....,IHasIndexer<int,T>
{
    ....
    public this[int key]
    {
        get {...}
        set {...}
        remove { this.Remove(key); }
    }
}

System.Collection.Generic.Dictionary<K,V> : .....,IHasIndexer<K,V>
{
    ....
    public this[K key]
    {
        get {...}
        set {...}
        remove { this.Remove(key); }
    }
}
...

remove list[0];
remove dict["key"];

Something like this. To use remove keyword it should explicitly contains remove accessor

Maybe this could be used on property too. If we have property that actually read things from other collection

class A
{
    ....
    Dictionary<string,int> serializeJsonMap;
    public int Removable
    {
        get { return serializeJsonMap["X"]; }
        set { serializeJsonMap["X"] = value; }
        remove { remove serializeJsonMap["X"]; }
    }
}

var a = new A();
remove a.Removable;

@iam3yal
Copy link

iam3yal commented Jan 11, 2017

@Thaina The call to the Remove method inside the indexers aren't really needed, the compiler can just map the signature of the indexer to an existing Remove method with the same signature but what's the benefit of this? this seems like introducing use-cases for the sake of introducing them but they aren't really useful and adding an indexer just to support this syntactic sugar seems more like a burden.

Why would you want to do remove dic["key"] when you can just do dic.Remove("key")?

@Thaina
Copy link
Author

Thaina commented Jan 11, 2017

I don't like the way compiler guessing Remove instead of explicit contract
Even using need IDisposable for calling Dispose am I wrong?

Now the difference between remove dic["key"] or dic.Remove("key") is just convention
Like linq could collection.Select((x) => something) or from collection select something and you can choose syntax you think it fit the project convention

Most of the times I use dic["key"] = value not dic.Add(value)
also var value = dic["key"] not var value = dic.GetValue("key")
so I would like to remove dic["key"] too

@iam3yal
Copy link

iam3yal commented Jan 11, 2017

@Thaina

I don't like the way compiler guessing Remove instead of explicit contract

Yes, fortunately it doesn't have to guess anything.

Now the difference between remove dic["key"] or dic.Remove("key") is just convention
Like linq could collection.Select((x) => something) or from collection select something and you can choose syntax you think it fit the project convention

I.. I don't know what to say.. you want to introduce such complexity just to replace a dot with a space!? and the funny part is that you actually believe that you can compare it to LINQ.

Most of the times I use dic["key"] = value not dic.Add(value)
also var value = dic["key"] not var value = dic.GetValue("key")
so I would like to remove dic["key"] too

Let me tell you how this reads: Most of the time I drive the car therefor I want to have the bus station near my house which doesn't make sense at all because they are completely unrelated not to mention being a reason for one of them to exist.

@Thaina
Copy link
Author

Thaina commented Jan 11, 2017

Then could you say why we have
from collection select x
just so you don't need to
collection.Select(x => x)
This only to replace dot capital and parentheses

I am the one who should said I don't know what to say for people who just cannot grow out of sql language and work with functional programming normally to the point that need to add from tobe a keyword

Your argument that you throw at me, all of that point to linq keyword in my perspective. Just because you want to transform collection with Select don't relate to adding from tobe a keyword at all

While what I was propose is to add convention standard to remove something. Like Dispose is standard to release resource

@Thaina
Copy link
Author

Thaina commented Jan 11, 2017

@eyalsk Also the problem using Remove like you say is it not work with List
List<T>.Remove() take T as argument, not int. And if it is List<int> then it cause ambiguous bug

That's why I call it guessing. Your argument > fortunately it doesn't have to guess anything < is invalid. There are also overload function aside List so we can't always apply things and hope it will retroactively work like that

ps. delete x["key"] and delete x[0] is javascript syntax

@iam3yal
Copy link

iam3yal commented Jan 11, 2017

@Thaina

This only to replace dot capital and parentheses

A piece of the puzzle might be insignificant on its own but the whole puzzle can make it worthwhile.

@Thaina
Copy link
Author

Thaina commented Jan 11, 2017

@eyalsk Then same go for remove keyword

The whole picture to have remove syntax is interop and serialization by dynamic and collection. The point is also to make consistent behaviour across dynamic language
there are del list[0] in python and delete list[0] in javascript. If you care about people adapt from SQL then I care more about people from python and js come to use C#

ps. I myself would not ever get the whole puzzle of linq keyword because I always prefer functional linq syntax over SQL

@iam3yal
Copy link

iam3yal commented Jan 11, 2017

@Thaina LINQ and SQL have different syntax and because both of them are declarative query languages they just happen to share some keywords but that's all.

Anyway, I feel like this discussion is a bit silly so we'll have to agree to disagree. 😄

@Thaina
Copy link
Author

Thaina commented Jan 11, 2017

@eyalsk Yes I think it silly from the start that you make argument without even knowing DynamicObject contains TryDeleteMember

@iam3yal
Copy link

iam3yal commented Jan 11, 2017

@Thaina No, it's silly to point out a mistake after someone already admitted it, it's silly to hang on to someone's mistake and use it as an argument in the absence of one.

@Thaina
Copy link
Author

Thaina commented Jan 12, 2017

@eyalsk I want you to know that your argument from the start make me feel it is silly argument. You admit it after does not make the past silliness get away

Just as I say I think it silly from the start, I mean till now I was feel all of your arguments is silly. I just don't say it because it rude. So I was trying to be reasoning about it as much as I can

But you say it out by yourself make me feel I don't need to bear it

@jnm2
Copy link
Contributor

jnm2 commented Jan 12, 2017

@Thaina I've noticed that it frequently happens in this kind of conversation that it seems to a person such as yourself that the others are being silly, when in fact they are being reasonable if you take a different perspective. @eyalsk's core argument is a good one, as is yours. Let's get away from the fringe metadiscussion.

@Thaina
Copy link
Author

Thaina commented Jan 12, 2017

@jnm2 I know it that's why I never start saying the word silly. I always try to understand perspective of other people

As I said I'm not start using word silly even I always feel it, I want to try to reasoning with it and make reasonable argument

Just that, whenever someone start using it, I too want to be honest about my feeling too

So, what I really want to say is, you should tell the person who start using it, not me

@iam3yal
Copy link

iam3yal commented Jan 12, 2017

@Thaina I DIDN'T say you are silly, I DIDN'T say your arguments are silly, I said that THE DISCUSSION is silly because it leads to nothing!

@Thaina
Copy link
Author

Thaina commented Feb 18, 2017

moved dotnet/csharplang#143

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants