RFC 7946 GeoJSON doesn't have special recognition for it anymore, and it makes things awkward because it's kinda redundant with Geometry.SRID.

I see that IAttributesTable is implemented as a Dictionary, which is not thread safe.

The code base I'm working with has a lot situations where Parallel.Foreach is used to loop through an IEnumerable<Feature>.

Within the loop, there are cases where IAttributesTable gets updated.

Is there a test I could run that would confirm that this can cause problems?

Assuming there is a reproducible issue, would implementing IAttributesTable with a ConcurrentDictionary work? Has anyone tried this?
Thanks!

Setup build similar to the core project.

During #8, I posited that it might be better to tweak IFeature and/or IAttributesTable to better model the two different use cases that IAttributesTable supports:

1. Some IAttributesTable instances have their set of valid attributes defined by something external to the individual instance (e.g., shapefiles, GPX, relational databases).
   • With these kinds of tables, you can tell when certain attributes are missing, and what type of data is they would need to hold when set.
2. Other IAttributesTable instances are more dynamic and can hold any data, or at least the system isn't supposed to reject anything outright (e.g., GeoJSON).
   • With these kinds of tables, you can add just about anything without errors.

From a reader's perspective, there's no difference between the two, but a writer who consume just the interface(s) would appreciate knowing what the rules are at compile-time.

Perhaps the interface(s) only need to support reading, and writing could be handled by using the concrete type directly. Alternatively, perhaps we can take a similar approach to what .NET did with its read-only collection interfaces, and expose a "read-only" base interface with different sub-interfaces that support the different kinds of writing.

Original question that sparked this issue:

Can IAttributesTable / AttributesTable be replaced with just IDictionary<string, object> directly on the Feature class?

That's basically all this is, anyway...

IFeatureCollection does not support Add(Object) so it's not easily used by the XmlSerializer.

1. Are there any examples of using the XmlSerializer with IFeatureCollection

2. If not how would one easily go about providing a general sterilizer if IFeatureCollection does not support Add(Object)...

The easiest thing I can think of at the current time is to serialize to use GML

In 1.x AttributesTable had a IDictionary<string, object>/HashTable constructor. In 2.x it was replaced by a Dictionary<string, object> constructor.

This change causes IDictionary<string, object> to use the IEnumerable<KeyValuePair<string, object>> constructor instead, so the Comparer of the IDictionary instance is lost. In my case, this is a comparer that ignores casing.

I've tried changing the constructor to accept an IDictionary again, but this breaks the Dictionary<string, object>.Enumerator GetEnumerator() method. That enumerator has a Current property, so changing it would also be a breaking change.

It's probably too late at this point to try and fix this, so perhaps the best thing to do here is document this change, what do you think?

Add a FeatureCollection constructor with a List parameter for quickly constructing a FeatureCollection instance based on some other list of Feature instances.

There was a similar constructor in the pre-2.0 version:

Now the only way to make such a FeatureCollection instance is an ugly foreach loop.

There are some important bug fixes in the latest 1.15.2 version of the NTS Core package. Can you please ship a new version that references that one?

GPX has a ton of well-defined attributes that can be present on each wpt, rtept, and trkpt element, but almost all of them are optional, and very few of them are actually specified on any given instance (different producers seem to favor different sets of them).

This makes it rather awkward to consume an IAttributesTable in order to produce elements from the GPX data model, since every optional value retrieved has to look like:

var feature = /* some feature */;
var attributes = feature.Attributes;
var timestampUtc = attributes.Exists(nameof(GpxWaypoint.TimestampUtc))
    ? (DateTime)attributes[nameof(GpxWaypoint.TimestampUtc)]
    : default(DateTime?);

I don't think this concept is necessarily limited to GPX (otherwise I'd just live with extension methods). It feels like this abstraction should simplify fetching optional attributes without requiring an extra virtual call and dictionary lookup for each one.

In my fantasy land, I can see myself writing:

var feature = /* some feature */;
var attributes = feature.Attributes;
var timestampUtc = (DateTime?)attributes.GetOptionalValue(nameof(GpxWaypoint.TimestampUtc));

The implementation would be a bit annoying:

public class AttributesTable
{
    /* ...*/
    public object GetOptionalValue(string attributeName)
    {
        object result = null;
        #if SERIALIZATION_COMPAT_NETTOPOLOGYSUITE_FEATURES_ATTRIBUTESTABLE
        // System.Collections.Hashtable doesn't have a way to skip this double-lookup
        if (_attributes.ContainsKey(attributeName))
        {
            result = _attributes[attributeName];
        }
        #else
        _attributes.TryGetValue(attributeName, out result);
        #endif

        return result;
    }
}

When creating a "regular" feature, serializing it and then deserializing it, I get a different behavior for GetOptionalId.
Here is the relevant test:

        [TestMethod]
        public void CheckGetOptionalIdEquality()
        {
            var options = new JsonSerializerOptions();
            options.Converters.Add(new GeoJsonConverterFactory());

            var feature = new Feature(new Point(0, 0), new AttributesTable { {"some-id", "my-id"} });
            var fullCycleFeature = JsonSerializer.Deserialize<IFeature>(JsonSerializer.Serialize(feature, options), options);
            Assert.AreEqual(feature.GetOptionalId("some-id"), fullCycleFeature.GetOptionalId("some-id"));
        }

This bit me hard in one of the releases I made :-(
I think it has to do with the IUniquId part of the STJ feature implementation.

NetTopologySuite/NetTopologySuite.IO.GeoJSON#117 was a bit of an awkward spot.

I would have preferred to have pointed to an IsReadOnly property and recommended copying the read-only IAttributesTable into a new editable instance, since I don't expect there to be many actual use cases where you need to edit the table in-place. After all, the table itself is little more than a way for us to expose the data that's present in the source file, and any edits that you make to the table instance do not directly get written back to that source file.

However:

• No IsReadOnly property (or related thing) exists, and so a negative response would have been unfair: it's unreasonable that "you just have to know" that the 4STJ implementation of IAttributesTable is read-only, especially when its Newtonsoft.Json counterpart did not work that way.
• There is no simple, built-in way to take an existing IAttributesTable instance and use it to initialize a new IAttributesTable object that's writable and has the same initial values, so even if had allowed myself to say "you just have to know", it would have still added an unreasonable amount of friction to the upgrade process, because a downstream consumer should not be expected to implement something like that on their own.

Things Not To Do... Yet...?

These would all be breaking changes:

1. add bool IsReadOnly { get; } directly to the IAttributesTable interface
2. add IAttributesTable ToWritable(); directly to the IAttributesTable interface
3. make an IReadOnlyAttributesTable interface, and change IAttributesTable to extend it

Suggestions

1. Create an IReadOnlyAttributesTable interface. Implement it in this project's AttributesTable class. When the new version of this library is published, implement that interface in the classes that implement this interface for all our other IO libraries.
2. Create a public static AttributesTable ToWritable(this IAttributesTable @this) extension method that creates a new empty AttributesTable and initializes it using values copied from @this.
   • Alongside this extension method, create a new interface, something like ICanCopyToNewWritableAttributesTable (better name???). It only has one method: void CopyTo(AttributesTable table);. The extension method would check if @this implements that interface. If it does, then we call that method. Otherwise, we do the copying ourselves. As above, implement this interface in all of our own implementations of IAttributesTable, for the best user experience.

I'm intentionally not jumping on this right away, because I think there's some room for some better ideas, and/or it might be a good first issue for someone new. (edit to add:) ...also, I still hold on hope that something might eventually happen with #6, which would affect this... even though, realistically, it's probably too big a change for too small a benefit...

Just tried to update to the NTS 1.15 Release, and after noticing Features were gone, I did some digging and finally stumbled across this :-P Was wondering when NetTopologySuite.Features will be pushed to Nuget as well?