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

Simplify fetching values for optional attributes in (I)AttributesTable #3

Closed
airbreather opened this issue Jul 28, 2018 · 1 comment · Fixed by #8
Closed

Simplify fetching values for optional attributes in (I)AttributesTable #3

airbreather opened this issue Jul 28, 2018 · 1 comment · Fixed by #8

Comments

@airbreather
Copy link
Member

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;
    }
}
airbreather added a commit that referenced this issue Jul 28, 2019
- All CoordinateSystems types are gone (resolves #7)
- (I)AttributesTable is gone, in favor of bare Dictionary<string, object> (resolves #6) (resolves #3)
- FeatureCollection now inherits from Collection<Feature>
- [Serializable] is now implemented via ISerializable
@FObermaier
Copy link
Member

I support this.

airbreather added a commit that referenced this issue Aug 23, 2019
- All CoordinateSystems types are gone (resolves #7)
- The extension methods dealing with ID values on IFeature instances are gone
    - to be moved to GeoJSON
- (I)AttributesTable now has GetOptionalValue (resolves #3)
- FeatureCollection now inherits from Collection<Feature>
- [Serializable] is now implemented via ISerializable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants