Iot: Property vs Function for reading: Update guideliness and device bindings

[krwq]

One of the offline feedback I got recently is that property when accessed twice in the row it should always produce the same value. We currently use properties for reading (i.e. Temperature) and that is not ture.

We should convert those to ReadTemperature - that should also make it easier to create async version of the code since the naming will be more consistent.

• [ ] Change all properties to Read* methods
  
  
  
  • [ ] Temperature
  
  
  • [ ] (placeholder for remainder: Pressure, ...)
  
  
• [ ] Make sure all async methods follow same naming convention (Read*Async)
• [ ] Update guideliness docs and all samples/readmes

FYI: @shaggygi @ZhangGaoxing @Ellerbach @richlander
cc: @bartonjs for guideliness comments

[bartonjs]

Yeah, the guidelines are there for it, they're just not cleanly in a bullet, so they didn't get transcribed.

Framework Design Guidelines, 2nd Edition, 5.1 p135-136:

• DO use a method, rather than a property, in the following situations:
  
  
  
  • The operation is orders of magnitude slower than a field access would be.
  
  
  • If you are even considering providing an asynchronous version of an operation to avoid blocking the thread [...].
    
    ...
  
  
  • The operation is a conversion, such as Object.ToString method.
  
  
  • The operation returns a different result each time it is called, even if the parameters don’t change. [...]
  
  
  • The operation has a significant and observable side effect. [...]
    
    ...
  
  
  • The operation returns a copy of an internal state (this does not include copies of value type objects returned on the stack).
  
  
  • The operation returns an array.
  
  

(and other reasons later, like "if it can throw an exception", but that's stated inversely as "AVOID throwing exceptions from property getters.")

[shaggygi]

I'm assuming our strategy on getting GpioPin values from properties needs to be changed, as well?

[krwq]

@shaggygi yes but we can change that separately

[Ellerbach]

I understand the rational for traditional elements but for sensors, I don't really see a value of having both a function and a property.
In the case of having both, if I understan,d correctly, a property to be a property should be Something like "LastTemperature" which will always return the last read temperature and it will be updated by a "ReadTemperature" async function. am I correct?
Also with all the sensors we currently have, it will be quitea lot of work to change in all the sensors.

[bartonjs]

In the case of having both, if I understan,d correctly, a property to be a property should be Something like "LastTemperature" which will always return the last read temperature and it will be updated by a "ReadTemperature" async function. am I correct?

The guidance isn't to have both, it's to rename the current get_Temperature() method (via the property) to GetTemperature() (and abandon the property).

[krwq]

@bartonjs would it be acceptable to have something like (I think this might be similar/same to what @Ellerbach proposed):

class SomeThermometer
{
  public void Read(); // possibly also ReadAsync
  public Temperature Temperature { get; } // last read temperature
}

so essentially pattern similar to what XML is doing - that should be a good trade-off between easy to use and clean, what do you think?

[bartonjs]

I think that everyone would get it wrong... unless the object is obviously an iterative reader, the notion of calling "Read" wouldn't occur to someone, so they'd just read the property, assume it's right, and have a bug later. (Maybe they'll find it quickly, but it's easy to make bad utility methods)

I really think that you want to just replace the property with a method. If a sensor has multiple properties that should be populated semi-atomically then "Refresh()" might make sense, but again, it's setting things up for people to get it wrong.

[krwq]

@bartonjs other reason for having Read would be that there are multiple sensors which read couple of values at the same time (i.e. temperature, humidity, pressure) - having Read would allow to not do 3 reads

[bartonjs]

there are multiple sensors which read couple of values at the same time (i.e. temperature, humidity, pressure) - having Read would allow to not do 3 reads

I'd call the void method that does that Refresh instead of Read, as mentioned above. But unless it's conforming to some sort of interface ITemperatureSensor : ISensor { SomeType Temperature { get; } }, I'd look at making one method that returns a struct of whatever it is that it's reading. (I say this without having any idea of what the models look like... I'm just sharing approach.)

It's very clear what happens when you have a method return a result.. it does something, and returns a result. It's a lot less clear when state comes "by magic". Double-reads of a property would still be an antipattern if any sensor had automatic refresh built-in.

Mainly it's to make sure that this is never broken:

```C#
if (sensor.Temperature >= TooHot)
{
Log.Error($"The current measured temperature ({sensor.Temperature}) has exceeded the high threshold ({TooHot}).");
}

If `Temperature` can change between reads, the log may say something like "The current measured temperature (55.8 C) has exceeded the high threshold (58.0 C).".

If you have to call Refresh/Read/Whatever then someone may completely forget to do that, and whatever temperature got recorded at startup is the only one ever used.  I can't imagine any scenario where having an inaccurate read of temperature would be bad [except possibly a really famous one](https://nssdc.gsfc.nasa.gov/planetary/lunar/ap13acc.html).

```C#
SomeType temperature = sensor.GetTemperature();

if (temperature >= TooHot)
{
    Log.Error($"The current measured temperature ({temperature}) has exceeded the high threshold ({TooHot}).");
}

No real ambiguity (the GetTemperature() method could internally have a "don't over-read" delay, but your answer is as up-to-date as the class and/or sensor are willing to give). You could get the same result with properties, and paranoid developers will write it that way... but it's really easy to write it the wrong way, and doesn't "look" wrong.

```C#
FancySensorResult result = sensor.GetData();

if (result.Temperature ...
```

also not ambiguous.

[ChadNedzlek]

I have to agree with Jeremy that we should cause people to fall into success unless they really try.

If something measures multiple things having

public struct MySensorReadings
{
    public int ValueA { get; }
    public int ValueB { get; }
}

then having a single way to get that

public class MySensor
{
    public MySensorReadings GetReadings();
}

Then it's really hard to use it wrong. It's unlikely you'd have cached values, since we forced you to do the right thing once, you don't have to learn a _new_ way to "do it right the second time"... the first, and correct, way is the only way. You would also be unlikely to have the double dipping problem Jeremy mentioned because

if (sensor.GetReadings().Temperature >= TooHot)
{
    Log.Error($"The current measured temperature ({sensor.GetReading().Temperature}) has exceeded the high threshold ({TooHot}).");
}

just looks awkward, so you'd be unlikely to write it that way. It looks really obvious in that chunk that are you "getting" a thing twice, so of course it'll be prone to change.

This could even lead itself to a pattern of something like

public interface ISensor<TReadings>
{
    TReadings GetReadings();
}

which would then cause people to very quickly learn the pattern that works right for all sensors. Though that be a bit much. I don't know what the full scope of all sensors in the universe is, and whether that level of easy pattern matching makes sense. (How many things return a set of values set against how many just return a single value, since single values would make this pattern sort of awkward)

[Ellerbach]

OK, so where it makes sense, we still can have Properties or where it's traditioanlly Properties (like in in the GPIO Controller) and for the rest, we will go for Functions.
Would be good then to document this a bit for everyone to be on the same page with a summary of the example you gave.
I like the Refresh pattern, the GetReading one as well as GetTemperature.

[shaggygi]

Change all properties to Read* methods

IMHO Read* fits better than Get*.

• Get/Set is usually a property value
• Read/Write is usually over the wire/file/etc for the value

Therefore, would it be more appropriate as @krwq mentioned to use ReadPressure, ReadTemperature, etc.?