NetTopologySuite / NetTopologySuite.Features

An implementation of Feature and FeatureCollection
BSD 3-Clause "New" or "Revised" License
8 stars 14 forks source link

Redesign IFeature / IAttributesTable to better model the different use cases it solves #6

Open airbreather opened 5 years ago

airbreather commented 5 years ago

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...

FObermaier commented 5 years ago

IAttributesTable has 7 function and a property to implement, IDictionary<string, object> has lots more.

For specific classes implementing IFeature I think the current approach is better.

airbreather commented 5 years ago

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

IAttributesTable has 7 function and a property to implement, IDictionary<string, object> has lots more.

For better or for worse, IDictionary<string, object> is the standard interface for the kind of object that we need, and IMO that outweighs the concern of wanting to keep things simpler for implementers.

Agreed, though, custom implementations of IDictionary<TKey, TValue> aren't easy. If it's important to support custom implementations, then perhaps we could help solve that by offering an abstract base class that implements the interface?

1 hour or so (completely untested) got me this, which only requires implementing 5 members:

public abstract class EasierDictionary<TKey, TValue> : IReadOnlyDictionary<TKey, TValue>, IDictionary<TKey, TValue>
{
    protected EasierDictionary()
    {
        Keys = new KeyCollection(this);
        Values = new ValueCollection(this);
    }

    public abstract int Count { get; }

    public abstract IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator();

    public abstract void Clear();

    protected abstract bool GetOrRemoveValue(TKey key, out TValue value, bool remove);

    protected abstract bool SetValue(TKey key, TValue value, bool onlyIfMissing);

    public KeyCollection Keys { get; }
    ICollection<TKey> IDictionary<TKey, TValue>.Keys => Keys;
    IEnumerable<TKey> IReadOnlyDictionary<TKey, TValue>.Keys => Keys;

    public ValueCollection Values { get; }
    ICollection<TValue> IDictionary<TKey, TValue>.Values => Values;
    IEnumerable<TValue> IReadOnlyDictionary<TKey, TValue>.Values => Values;

    public IEqualityComparer<TKey> KeyComparer { get; set; }

    public IEqualityComparer<TValue> ValueComparer { get; set; }

    public TValue this[TKey key]
    {
        get => GetOrRemoveValue(key, out var value, false) ? value : throw new KeyNotFoundException();
        set => SetValue(key, value, false);
    }

    public bool ContainsKey(TKey key) => GetOrRemoveValue(key, out _, false);

    public void CopyTo(KeyValuePair<TKey, TValue>[] array, int arrayIndex)
    {
        foreach (var kvp in this)
        {
            array[arrayIndex++] = kvp;
        }
    }

    public void Add(TKey key, TValue value)
    {
        if (SetValue(key, value, true))
        {
            throw new ArgumentException("", nameof(key));
        }
    }

    public bool Remove(TKey key) => GetOrRemoveValue(key, out _, true);

    public bool TryGetValue(TKey key, out TValue value) => GetOrRemoveValue(key, out value, false);

    void ICollection<KeyValuePair<TKey, TValue>>.Add(KeyValuePair<TKey, TValue> kvp) => Add(kvp.Key, kvp.Value);

    bool ICollection<KeyValuePair<TKey, TValue>>.Contains(KeyValuePair<TKey, TValue> item)
    {
        if (!GetOrRemoveValue(item.Key, out var value, false))
        {
            return false;
        }

        var valueComparer = ValueComparer;
        if (default(TValue) == null)
        {
            // https://github.com/dotnet/coreclr/issues/17273
            valueComparer = valueComparer ?? EqualityComparer<TValue>.Default;
        }

        return valueComparer is null
            ? EqualityComparer<TValue>.Default.Equals(item.Value, value)
            : valueComparer.Equals(item.Value, value);
    }

    bool ICollection<KeyValuePair<TKey, TValue>>.Remove(KeyValuePair<TKey, TValue> item)
    {
        if (!TryGetValue(item.Key, out var value))
        {
            return false;
        }

        var valueComparer = ValueComparer;
        if (default(TValue) == null)
        {
            // https://github.com/dotnet/coreclr/issues/17273
            valueComparer = valueComparer ?? EqualityComparer<TValue>.Default;
        }

        if (valueComparer is null)
        {
            if (!EqualityComparer<TValue>.Default.Equals(item.Value, value))
            {
                return false;
            }
        }
        else
        {
            if (!valueComparer.Equals(item.Value, value))
            {
                return false;
            }
        }

        return GetOrRemoveValue(item.Key, out _, true);
    }

    bool ICollection<KeyValuePair<TKey, TValue>>.IsReadOnly => false;

    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

    public sealed class KeyCollection : ICollection<TKey>
    {
        internal readonly EasierDictionary<TKey, TValue> _dict;

        public KeyCollection(EasierDictionary<TKey, TValue> dict) => _dict = dict;

        public int Count => _dict.Count;

        public void CopyTo(TKey[] array, int arrayIndex)
        {
            foreach (var kvp in _dict)
            {
                array[arrayIndex++] = kvp.Key;
            }
        }

        public Enumerator GetEnumerator() => new Enumerator(_dict);

        bool ICollection<TKey>.IsReadOnly => true;
        IEnumerator<TKey> IEnumerable<TKey>.GetEnumerator() => GetEnumerator();
        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
        bool ICollection<TKey>.Contains(TKey item) => _dict.ContainsKey(item);
        void ICollection<TKey>.Add(TKey item) => throw new NotSupportedException("");
        void ICollection<TKey>.Clear() => throw new NotSupportedException("");
        bool ICollection<TKey>.Remove(TKey item) => throw new NotSupportedException("");

        public struct Enumerator : IEnumerator<TKey>
        {
            private readonly IEnumerator<KeyValuePair<TKey, TValue>> _enumerator;

            internal Enumerator(EasierDictionary<TKey, TValue> dict) => _enumerator = dict.GetEnumerator();

            public TKey Current => _enumerator.Current.Key;
            object IEnumerator.Current => Current;

            public void Dispose() => _enumerator.Dispose();
            public bool MoveNext() => _enumerator.MoveNext();
            public void Reset() => _enumerator.Reset();
        }
    }

    public sealed class ValueCollection : ICollection<TValue>
    {
        private readonly EasierDictionary<TKey, TValue> _dict;

        internal ValueCollection(EasierDictionary<TKey, TValue> dict) => _dict = dict;

        public int Count => _dict.Count;

        public void CopyTo(TValue[] array, int arrayIndex)
        {
            foreach (var kvp in _dict)
            {
                array[arrayIndex++] = kvp.Value;
            }
        }

        public Enumerator GetEnumerator() => new Enumerator(_dict);

        bool ICollection<TValue>.IsReadOnly => true;
        IEnumerator<TValue> IEnumerable<TValue>.GetEnumerator() => GetEnumerator();
        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

        bool ICollection<TValue>.Contains(TValue item)
        {
            var valueComparer = _dict.ValueComparer;

            if (default(TValue) == null)
            {
                // https://github.com/dotnet/coreclr/issues/17273
                valueComparer = valueComparer ?? EqualityComparer<TValue>.Default;
            }

            if (valueComparer is null)
            {
                foreach (var kvp in _dict)
                {
                    if (EqualityComparer<TValue>.Default.Equals(kvp.Value, item))
                    {
                        return true;
                    }
                }
            }
            else
            {
                foreach (var kvp in _dict)
                {
                    if (valueComparer.Equals(kvp.Value, item))
                    {
                        return true;
                    }
                }
            }

            return false;
        }

        void ICollection<TValue>.Add(TValue item) => throw new NotSupportedException("");
        void ICollection<TValue>.Clear() => throw new NotSupportedException("");
        bool ICollection<TValue>.Remove(TValue item) => throw new NotSupportedException("");

        public struct Enumerator : IEnumerator<TValue>
        {
            private readonly IEnumerator<KeyValuePair<TKey, TValue>> _enumerator;

            internal Enumerator(EasierDictionary<TKey, TValue> dict) => _enumerator = dict.GetEnumerator();

            public TValue Current => _enumerator.Current.Value;
            object IEnumerator.Current => Current;

            public void Dispose() => _enumerator.Dispose();
            public bool MoveNext() => _enumerator.MoveNext();
            public void Reset() => _enumerator.Reset();
        }
    }
}
shmuelie commented 5 years ago

I spent some time thinking on this, with the hardest part being "what is a feature". After reading too many OGC docs, I found that a "Feature" is defined as part of GML, section 9.

If we go by what GML says then a Feature is simply an object, with some optionally predefined properties and a geometry.

I almost feel like IFeature could just have a geometry and an IReadOnlyDictionary<string, object> for properties. For things that have predefined properties additional interfaces or just implementations would work. so something like:

interface IFeature
{
    Geometry Geometry { get; set; }
    IReadOnlyDictionary<string, object> Attributes { get; }
}

// For GPX for example
interface IWaypointFeature : IFeature
{
    int Elevation { get; set; }
    DateTimeOffset Time { get; set; }
    // ...
}