I'd merge it, but I won't pretend I understand the regex 😊

Can you add some unit tests for ObjectSerializer which prove the behaviour?

Sure. I think I'll also rename it to JsonNetObjectSerializer, since this is actually bleeding through the abstraction IObjectSerializer.

The regex basically says "one or more characters, followed by either $ref or $id, wrapped in quotes, followed by one or more characters and a newline" 😄

If I run the following script using my ObjectDumper script pack, I'm sure it provides what you are after:

var dumper = Require<ObjectDumper>()
    .PrettyPrint()
    .Build();

public class Person  {
    public string FirstName {get; set;}
    public string LastName {get;set;}
    public Address Address1 {get;set;}
    public Address Address2 {get;set;}
}
public class Address {
    public string Line1 {get;set;}
    public string Line2 {get;set;}
}

var address = new Address { Line1 = "Line1" };
var person = new Person { FirstName = "Paul", LastName = "Bouwer", Address1 = address, Address2 = address};

Console.WriteLine(dumper.Dump(person));

This is just using settings on the JSON.Net JsonSerializerSettings object. If this is what you need then there's no need to perform additional regex after serialisation.

Added two tests. Confirmed both failed without regex 😄

@paulbouwer How do you propose we set up our serializer to omit the $ref and $id properties? We have to be able to serialize references, including circular ones. See #553 for discussion.

Having read through the related discussions it appears that we do need to allow the serializer to add the $id and $ref properties for it to work correctly with circular references.

@khellang oh wow, good thinking!

@khellang Sorry missed the circular reference part. Agreed @adamralph - that doesn't work without $ref and $id being rendered ...

Do we lose too much context when removing $ref via the regex ? Or have I serialised my example below incorrectly ?

before regex:

{
  "$id": "1",
  "FirstName": "Paul",
  "LastName": "Bouwer",
  "Address1": {
    "$id": "2",
    "Line1": "Line1",
    "Line2": null,
    "Person": {
      "$ref": "1"
    }
  },
  "Address2": {
    "$ref": "2"
  }
}

after regex:

{
  "FirstName": "Paul",
  "LastName": "Bouwer",
  "Address1": {
    "Line1": "Line1",
    "Line2": null,
    "Person": {
    }
  },
  "Address2": {
  }
}

@paulbouwer you may have a valid point here. It seems like $id and $ref aren't just noise.

For circular refs, I guess there is no other way to represent them as JSON other than a $ref.

For repeated objects, as in your address example, I would hope there is a way to tell JSON.NET to repeat the objects rather than $ref them. It's verbose and redundant information but I think it's far nicer to read. I wonder if @JamesNK can chip in here?

Perhaps we should take another approach:

1. after serialization, remove all $id which do not have a corresponding $ref.
2. investigate whether, for non circular $ref's, the objects can be repeated instead of being $ref'd

Oh I see the issue now, this is a good catch @paulbouwer. I feel like we are obsessing over this more than we need to. We talked about this before and decided NOT to pursue it because when we spoke to @JamesNK he described why there were needed.

We should roll this commit back imo.

@adamralph I really don't feel like this is worth a lot of effort. Let's just accept $id and $ref and move on

OK talking with @adamralph I think we can reasonably go ahead and use regex to find all $id and all $ref. Then do a comparison to find which $id have no $ref and remove those. This would keep $id ONLY when $refs were present.

IMO you should serialize directly to LINQ to JSON objects, navigate through the object tree and remove the $id properties that don't have a corresponding $ref, then ToString() to get the JSON and print it. Much better than using regex over strings. I think I suggested this when I got brought into a scriptcs discussion about $ref/$id for the 500th time!

If you don't know how to do any of that I refer you to the documentation.

Please just figure it out because it feels like ground hog day over here.

Where I described what you should do: #553 (comment)

And for bonus points, a comment where I described how to get rid of $id and $ref all together (which is what you originally had!) without a regex: #553 (comment)

I think I am destined to live in a world where every 3 months scriptcs will alternate between wanting $id and $ref properties and not wanting them, and every time you'll ask me what to do.

I agree with JamesNK on this. Let's use the API that JSON.NET provides. Going to all the way to a string and then regex'ing just doesn't make sense.

Having said that, I don't think this is a huge priority. I've raised #700 for this.

In the meantime we need to revert this PR.

raised #701 to revert

What if it is is a deep object graph? Wouldn't the regex cheaper then
having to recursively walk the graph which includes checking each object to
see if if has array properties?

On Monday, May 19, 2014, James Newton-King notifications@github.com wrote:

IMO you should serialize to LINQ to JSON objects, navigate through the
object tree and remove the $id properties that don't have a corresponding
$ref, then ToString() to get the JSON and print it. Much better than using
regex over strings. I think I suggested this last year when I got brought
into a scriptcs discussion about $ref/$id for the 500th.

If you don't know how to do any of that I refer you to the documentation.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/696#issuecomment-43481416
.

You may have a point. I guess if anyone feels the inclination to pick up #700 then they can run some tests.

@adamralph I'll pick up #700 - would like to fold some of those features into my ScriptCs.ObjectDumper script pack anyway.

Awesome, thanks!
On 20 May 2014 08:26, "Paul Bouwer" notifications@github.com wrote:

@adamralph https://github.com/adamralph I'll pick up #700https://github.com/scriptcs/scriptcs/issues/700\- would like to fold some of those features into my ScriptCs.ObjectDumper
anyway.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/696#issuecomment-43590388
.