Is there a more elegant way to check highest / lowest sample data?

Question

N00b question,

Is there a better way for readability / elegance / DRY etc to check various sensor values for high/low record value? This code works, but looks like the long way round.

  void checkRecord() {

if ( temp.current > temp.high) {
  temp.high = temp.current;
}

if ( temp.current < temp.low) {
  temp.low = temp.current;
}

if ( pressure.current > pressure.high) {
  pressure.high = pressure.current;
}

if ( pressure.current < pressure.low) {
  pressure.low = pressure.current;
}
if ( humidity.current > humidity.high) {
  humidity.high = humidity.current;
}

if ( humidity.current < humidity.low) {
  humidity.low = humidity.current;
}

if ( rainfall.current > rainfall.high) {
  rainfall.high = rainfall.current;
}

if ( rainfall.current < rainfall.low) {
  rainfall.low = rainfall.current;
}
if ( altitude.current > altitude.high) {
  altitude.high = altitude.current;
}

if ( altitude.current < altitude.low) {
  altitude.low = altitude.current;
}
}

Thanks!

Answer 1

This is a variation on the struct idea with an object-oriented flavor.

If you put the current, low and high values of any sensor inside a struct, you could also morph this struct into a class and give it an update() method that, in addition to updating the current value, keeps the low and high updated:

class LowHigh
{
public:
    LowHigh() : current(0), low(INFINITY), high(-INFINITY) {}
    void update(float value) {
        current = value;
        if (current < low)
            low = current;
        if (current > high)
            high = current;
    }
    float get_low() { return low; }
    float get_high() { return high; }
private:
    float current, low, high;
};

I am assuming float data here, you may have to adapt to whatever type is appropriate for your sensors.

You would use the class as this:

LowHigh temp, pressure, humidity, rainfall, altitude;

void loop()
{
    temp.update(temp_sensor.get_reading());
    pressure.update(pressure_sensor.get_reading());
    humidity.update(humidity_sensor.get_reading());
    // ...
}

Answer 2

You could write something like:

temp.high = (temp.current > temp.high) ? temp.current : temp.high;
temp.low  = (temp.current < temp.low) ? temp.current : temp.low;
...

Most of the time there are min/max macros defined already like:

#define MAX(a, b)   ((a > b) ? a : b)
#define MIN(a, b)   ((a < b) ? a : b)

So you can then simply write:

temp.high = MAX(temp.current, temp.high);
temp.low  = MIN(temp.current, temp.low);
...

Which expands to the exact code as given above.

Answer 3

If the temperature, pressure, humidity, rainfall, and altitude structs were elements of an array of structs, you could iterate over those elements setting each one's high and low values with only two handwritten 'if-then' statements. If you also used an enum to name the indices of the structs, then in other parts of your code you could address each of them by a descriptive name:

// Declare a data struct and define an array of them.
struct {
   int16_t current;
   int16_t high;
   int16_t low;
} data[5] = {0, 0, 0, 0, 0};

// Name the structs by index
enum kind {
   temperature = 0,
   pressure,
   humidity,
   rainfall,
   altitude
};


// Assign one kind of data:
data[temperature].current = getTermperature();


// Set maxes & mins for all of the kinds of data:
for( i = 0; i < sizeof(data)/sizeof(data[0])); ++i ){
   if ( data[i].current > data.high) {
      data[i].high = data[i].current;
   }

   if ( data[i].current < data[i].low) {
      data[i].low = data[i].current;
   }
}

Update:

I'm strugling with the data[5] {0,0...} part in struct.

I can't see your whole sketch but you probably have allocated 5 separate structs named "temp", "pressure", ... , etc. I'm suggesting you allocate an array of 5 structs (which I collectively called "data"), one for each of your individual ones. I included an initializer ( '= {0,0,0,0,0}') to set everything to zero before the start of the run.

all of enum kind{} and how it becomes data[pressure] etc.,

The enum is a quick way to make 5 symbols that stand for consecutive integers, in this case, the index into the array of each struct, using a descriptive name for that struct's data. So 'data[pressure]' is the same as data[1] (in this case) but is more understandable.

the sizeof(data)/sizeof(data[0]) in the loop

That expression gives the number of structs in the array (the total byte-size of the array divided by the byte-size of one struct). We could write '5' instead, but when you someday add another member or two (wind_direction, wind_speed, perhaps?), you'd have to remember to also fix your for() loop. By using this calculation, the compiler will do it for you, leaving you one less opportunity to make a mistake you'd then need to debug.

and finally I think altitude.high in the if statement is a paste typo? should be data[i].high?

Absolutely right! Fixed now.

Update:

For defining initial values can you just assign an initial value in the struct such as int16_t low = INFINITY; and so on instead of {0,0,0,0,0}?

Yes - Without knowing your data and application, I chose the simplest initializer. The full syntax to do what you want would look like this:

// Declare a data struct and define an array of them.
struct {
   int16_t current;
   int16_t high;
   int16_t low;
} data[5] = {
  {0, 0, INFINITY},
  {0, 0, INFINITY},
  {0, 0, INFINITY},
  {0, 0, INFINITY},
  {0, 0, INFINITY}
};

You don't need (and may not) specify each data type and name; the struct declaration already provides that information. The initializers are positional and each one must be compatible with the data-type it matches up to.