Added rudimentary +,-, >, <, == operators for DateTime objects and tests

[namanvs]

I have added <, >, and == operator overloading for the DateTime class.

These comparison operators compare the year, month, day, hour, minute and seconds fields directly, because unixtime only matches if the timezone is set to UTC.

I have also added + and - operators with the following behaviors:

• operator takes a DateTime (LHS) and a number of seconds (RHS) and returns a new DateTime that many seconds greater than the LHS. The RHS must be >= 0, because adding negatives causes underflow with the underlying UnixTime variable.

• operator returns the difference in seconds between two DateTimes. This value can be negative, meaning LHS is chronologically before the RHS.

[IowaDave]

@namanvs , Thank you for contributing! I look forward to reviewing your code.

To you and @awickert : Please allow me a few days to complete work on an initial set of documentation for existing functions before reviewing these DateTime enhancements. It will not take long. The writing is basically finished. I am in that pre-push phase of self-criticism and fine-polishing. I might include a couple of example programs for working with the timer outputs on the 32K and the INT/SQW pins.

By the way, I have already uploaded and merged some documentation on the DateTime class. Based on testing after some recent PRs, it seems to me that the issues raised in the past about this class have been addressed and resolved. Please take a look at it in the "Documentation" folder. I will be grateful for the benefit of your thoughts.

David

[IowaDave]

Hi @namanvs
cc: @awickert

I had some time on Sept 17 to look at the operator overloadings in this PR.

Must confess I might not be the best person to assess operator overloading.

Looking at the implementations, I found a lot in this PR that might benefit from further study. Maybe best to take it one thought at a time.

For example...

The range of dates supported by DateTime may warrant consideration for the "+" overload function. See the 2106 Problem. Perhaps some added code might be warranted to constrain the valid range of the return value?

Thanks, David

[IowaDave]

I am trying to understand the logic in the ">" and "<" overloads.

We have expressions such as the following

this->year() > rhs.year() || this->month() > rhs.month() || ...

which could be abstracted as boolYear || boolMonth || ....

Each of the six comparisons can evaluate to (true) or (false). The six values are consolidated into a single boolean value by applying successive logical OR operators to them, one after the other.

I ran a simulation assigning (true) and (false) randomly to six booleans then performing the same calculation. A series of trials of six booleans and the result of sequential "OR" operations on them is shown below. Is this the desired outcome?

If not, what changes to the code for these two overloaded operators could ensure correct comparisons of two DateTime instance objects?

Six boolean values       result
---------------------    ------
0 : 0 : 1 : 0 : 1 : 0       1
0 : 0 : 1 : 1 : 0 : 0       1
1 : 1 : 0 : 1 : 1 : 1       1
0 : 1 : 1 : 0 : 0 : 0       1
0 : 0 : 0 : 0 : 0 : 0       0
1 : 0 : 1 : 1 : 1 : 0       1
1 : 0 : 0 : 0 : 1 : 0       1
0 : 1 : 1 : 0 : 0 : 0       1
1 : 0 : 1 : 1 : 1 : 0       1
0 : 0 : 0 : 1 : 0 : 0       1
1 : 1 : 0 : 1 : 0 : 0       1
1 : 1 : 0 : 1 : 1 : 0       1
0 : 0 : 0 : 1 : 0 : 0       1
0 : 1 : 0 : 1 : 0 : 1       1
0 : 1 : 1 : 0 : 0 : 0       1
0 : 1 : 0 : 1 : 1 : 0       1
1 : 1 : 1 : 1 : 0 : 0       1
1 : 1 : 1 : 1 : 0 : 1       1
1 : 0 : 1 : 1 : 0 : 1       1
0 : 1 : 0 : 0 : 1 : 1       1
0 : 0 : 1 : 0 : 1 : 1       1

By the way, the unixtime() method of the DateTime class does not take into account the setting of a "system clock" nor does it access any information about UTC or any other time zone.

I believe that the only necessary condition for comparison to be valid between two DateTime objects is that the program writer ensures the values they contain reflect the same time zone, which need not be UTC.

If my belief is correct, then might these comparison overloads simply examine the unixtime() results of the two DateTime objects?

[namanvs]

My initial goal with this PR was to add richer comparison operations for DateTime objects that let the programmer think in terms of calendar units (minutes, hours, days, months and years) instead of always having to drill down to the unix time uint32s, but it seems like I bit off more than I can chew, especially when considering issues like how months have differing numbers of days. Perhaps directly comparing unix time is the easier approach, especially if we include timezone handling, which I am personally all for, but otherwise, the comparators as they stand basically provide a deeper comparison of DateTime classes.

My intention with the '>' and '<' was to use the short-circuiting behavior of chained OR checks. Each expression is evaluated from left to right, so the first OR that evaluates as TRUE stops the evaluation of subsequent OR checks. Therefore setting the precedence of each check in order of largest to smallest unit of time should result in the expected behavior.

For example, A>B?

First we check A.year() > B.year():
If A.year() > B.year(), then A > B, so the evaluation stops and returns TRUE. Otherwise, we are still not sure of the result of the A>B, so we check the next OR condition, which is A.month() > B.month(), and so on...

In your test of 6 random booleans, because of short-circuiting, only the first boolean that evaluates to TRUE causes the result to evaluate as TRUE. The rest are actually of no consequence. However, in any case your table of results is the desired outcome.

I was mulling over creating a Duration class to facilitate much more natural DateTime operations, e.g:

Duration has Years, Months, Days, Hours, Minutes, Seconds, and

DateTime - DateTime = Duration

DateTime + Duration = DateTime

DateTime % Duration = Duration

But all this seemed to get too hairy and too complex for something running on a microcontroller. At least the current implementations of +. -, >, <, == should provide some utility for people creating software timers ( + can be used to set the next time an event should fire), measuring time elapsed ( - operator), or checking if a particular DateTime has been reached or exceeded with the comparators (<,>,==).

I could also easily extend the set of operators by composing:

A >= B can be written as A > B || A ==B

and A != B as !(A==B)

but I'll leave this decision to you.

[IowaDave]

This is a good and thoughtful conversation. Thank you! Let's keep it going.

I appreciate your point that the absence of time zone information somewhat limits the DateTime class. Well, that might be the way things are at the moment.

Given the limitation we would need to emphasize to users that operations on two DateTimes are valid only in the situation where both operands represent time in the same time zone.

I came in here to add some thoughts about two more operators.

Firstly, my thought on the "==" overload would take me back to suggesting comparison of the two unixtime() method outputs, given the lack of access to time zone information.

Also, the "-" overload looks fine to me. Again, the return value embeds the assumption of identical time zone.

I really want to understand the logic better in the ">" and "<" overload functions. It appears that you and I might be interpreting the order of computation differently. My understanding is that a calculation written thus:

return A || B || C || D || E || F; will always evaluate all six values. It must do so, because logical OR is commutative, meaning that changing the order of operation does not change the result. It might even be possible that the compiler would not guarantee to perform the calculations in the same order as written in a long sequence like that.

But I do see that you interpret it differently.

I believe you and I conceive the the intended logic of the matter similarly, even if we might elect different syntax to encode it. Will you allow me to attempt your idea in my words, so we can look at it together?

Something along the following line of pseudocode would illustrate my understanding of the logic result you described for two DateTime instance objects A and B:

if (A.year > B.year) return true; // end of comparison because A > B for sure
if (A.year < B.year) return false; // another, definite end of comparison because A < B for sure
// otherwise, year == year and  we don't know yet, need to check month next.

if (A.month > B.month) return true; // done!
if (A.month < B.month) return false; // likewise done!
// otherwise, year == year and month == month, we still don't know, check day...

if (A.day > B.day) return true; 
if (A.day < B.day) return false;
// otherwise, year == year and month == month and day == day
... and so onward to hour, minute and second

All that lovely logic having been exercised, would the program writer using the comparison operator even know whether the implementation was performed with timestamps, rather than with six successive sets of comparisons?

[IowaDave]

I hasten to add, regarding the previous post, that the pseudocode listing at the end of it was for the ">" overload. The same logic would work for the "<" overload function, but reversing the return values.

[IowaDave]

I compile the following code and receive a result that is contrary to what @namanvs was hoping.

Notice that only the "month" value of A is greater than its counterpart in B. All other values in A are either less or equal to those in B. In particular, the "year" in A is less than that in B.

Yet, alas...

#include <DS3231.h>
DateTime A(2021, 5, 25, 6, 0, 0);
DateTime B(2022, 4, 26, 7, 0, 0);
void setup() {
  Serial.begin(9600);
  bool C = A > B;
  if (C) {
    Serial.println("A is greater than B");
  } else {
    Serial.println("A is NOT greater than B");
  }
}

Result: "A is greater than B".

This result appears to support a contention that Logical OR is commutative and the ">" overload function as written evaluates all six date and time element comparisons.

[namanvs]

Well, I feel like a dunce for misunderstanding how short-circuit behavior works...

How about this? I will re-work this code so that comparators simply look at UNIX time. I will add support for >=, <=, and != as well.

Regarding timezone support. It might just be easiest to add an enum to the DateTime class with all the UTC offsets, and have it as an optional parameter in all the constructors (defaults to 0, i.e. UTC time). Calling the unixtime() function will report the current UNIX time in UTC. I.e. if your timezone was UTC +4, unixtime() would subtract 4 hours from the value obtained after adding SECONDS_FROM_1970_TO_2000.

[IowaDave]

@namanvs Oh! I know how you feel. Want to see a really big collection of dunce hats? Take a look at mine!

Iteration is how we make open source software. It is also how we all learn. Who never errs, has stopped trying. Who never loses the path, gains no knowledge of the world it passes through.

Let's take things a step at a time. A robust set of operator overloads on the existing data structure of the class could add some value. Please do complete those, as you propose.

The idea of altering the data structure to introduce time zones best belongs in a separate PR.

Many thanks,

David

[IowaDave]

@namanvs : Please also strengthen the "+" operator overload function to guard against sums exceeding the design range of the DateTime class.

Firstly, it is necessary, but not sufficient, to guard against overflowing the 32-bit unsigned integer return type.

Secondly, the sum of this->unixtime() + rhs should be protected against overflowing the 8-bit year-offset (yOff) property of the class. This means the maximum sum must not exceed 4102444799 = 23:59:59 (p.m.) December 31, 2099.

Note that a sum of 4102444800 would overflow the yOff property to 00 from 99. A new DateTime instance constructed on such a sum would have no meaningful value in relation to the two parameters fed into the "+" overload function.

The "+" operator overload should implement appropriate safeguards against sums exceeding the value 4102444799.

[IowaDave]

@namanvs Today it is my face that turns red with embarrassment. Please allow me to modify my request.

In the previous comment I remarked that a Unix-style timestamp value of 4102444800 would overflow the yOff property of the DateTime class to 00 from 99. That is not correct.

The yOff value will be set to 100, not zero. Note that the DateTime year() method will return 2100 because it adds 2000 to the yOff value.

Likewise, the getYear() method of the DS3231 class will return 100 after setting the DS3231 hardware with setEpoch(4102444800).

It might not be strictly necessary to constrain timestamp sums to values less than 4102444800. Even so, I believe it still makes sense to prevent overflowing the 32-bit integer. The maximum timestamp comparable to that of the present day would be 2³² – 1 = 4294967295 ( = 06:28:15 Feb 7 2106).

Thanks,

David

[IowaDave]

@namanvs
Couple of comments...

UINT32_MAX is already defined, for AVR architecture at least, in the core library stdint.h.

It surprised me to observe that the compiler would not object to assigning an integer constant, e.g., 4294967295, to a DateTime class instance. The following is essentially what would happen in your proposed code:

DateTime myDT = 4294967295;

... and it compiles without so much as a warning! That strikes me as very odd.

Even more puzzling to me, that instruction executed quietly when I tested it on a Nano.

Wearing my documentation-writer hat, let me ask you two questions:

What time, date, and unixtime() values did you expect to result in the DateTime instance following this assignment, and what values did you actually find?

How should users of this overload function formulate their code to detect the "overflow" result?

[IowaDave]

@namanvs
It seems likely that the compiler behavior described in the previous post results from having a constructor that accepts a single integer argument.

Consideration is being given to marking that constructor "explicit", which may prompt the compiler to regard assigning an integer as an error.

To prepare for this possibility, and for compatibility with the function's declared return type, please consider modifying your "+" overload function to return a datetime object rather than an integer. For example, using the value you propose,

"return DateTime(UINT32_MAX);.

It would then be possible to tell users what date and time values to expect in the event of an overflow. The community can decide later whether such a result is what they want.

Thanks!

[IowaDave]

@awickert @jnuernberg @hasenradball
I am asking others to step in and take over the consideration of this PR #71 for two reasons.

Firstly, while it remains under development, unfortunately I need to reduce the amount of time I devote to this repo. The PR will move forward more quickly if someone else steps in.

Secondly, I don't feel qualified to judge how much value the PR adds to the library. The "+" operator as written looks problematic to me. The other operator overload functions boil down to one-line code statements that a library user could as easily write directly.

Thanks, David

[hasenradball]

Hi David,

I will have a look in the PR the check what the Topic is abaout.

[hasenradball]

Why reinvent the wheel twice, and defining the uint32_MAX separately?
I would use the value of the header climits.
See climits.h

[hasenradball]

In general you can think about to use the function description like this:

/**
 * @brief description of function
 * 
 * @param parameter 1 description
 * @param parameter 2 description
 * @return description of return value
 */

Then you can hover over the function and it will display the description.
See Document functions
You will see this benefit in VScode or in Arduino IDE 2.0.

[hasenradball]

I think returning the MAX value here is misleading in a way.
I would recommend here normally to return an error, but this might be difficult here.
Thus I would return DataTime() ?> 0.