gost / sensorthings-net-sdk

.NET SDK for OGC SensorThings
MIT License
2 stars 2 forks source link

Mixed functions with data transfer objects #21

Closed DLade closed 3 years ago

DLade commented 4 years ago

There are several functions using the client inside the DTOs (data transfer objects). It might be convenient to have such functions there but from an architecture view it is not "nice".

Solution 1 (my preference, much less code): The client can do it:

client.Get<SensorThingsCollection<Observation>>>(datastream.ObservationsNavigationLink)

or (SensorThingsEntityHandler style)

client.GetEntities<Observation, Datastream>(datastream)

Solution 2: Extensions outside the DTOs can hadle it:

namespace sensorthings.Extensions {
    public static class DatastreamExtensions {
        public static async Task<Response<SensorThingsCollection<Observation>>> GetObservations(
            this Datastream datastream, SensorThingsClient client, OdataQuery odata = null) {
            if (!string.IsNullOrEmpty(datastream.ObservationsNavigationLink)) {
                return await Http.GetJson<SensorThingsCollection<Observation>>(datastream.ObservationsNavigationLink);
            }
            return await client.GetObservationCollectionByDatastream(datastream.Id, odata);
        }
        ...
}

Solution 3: Mix both:

namespace sensorthings.Extensions {
    public static class DatastreamExtensions {
        public static async Task<Response<SensorThingsCollection<Observation>>> GetObservations(
            this Datastream datastream, SensorThingsClient client, OdataQuery odata = null) {
                return await client.GetEntities<Observation, Datastream>(datastream, odata);
        }
        ...
}

What do you think? Which solution do you like most?

DLade commented 4 years ago

With "from an architecture view it is not nice" I mean that one should split data and function to prevent a strong coupling between components/classes. Decoupling things make maintenance much easier.

Functions which needs the client should be part of the client or like in C# at least extensions.

But I see not much efford in calling thing.getObservations(client) instead of client.getObservations(thing).

You know what I mean?

bertt commented 4 years ago

Hi, just took a quick look, at first sight the solution 1 looks nice to me

client.Get<SensorThingsCollection<Observation>>>(datastream.ObservationsNavigationLink)
DLade commented 3 years ago

If we merge this, we should change the major number - because the interface changes a bit.